From c7099eb1c19e60251e6725d6302354dfabae5303 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Fri, 13 Jun 2014 13:04:36 +0300 Subject: [PATCH 1/5] ASoC: simple-card: Make u32 DT parameter handling 64-bit proof Passing unsigned int pointers as u32 ponters may be dangerous on 64-bit system. Signed-off-by: Jyri Sarha Signed-off-by: Mark Brown --- sound/soc/generic/simple-card.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 03a7fdcdf114..159e517fa09a 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -116,6 +116,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np, { struct device_node *node; struct clk *clk; + u32 val; int ret; /* @@ -151,10 +152,8 @@ asoc_simple_card_sub_parse_of(struct device_node *np, } dai->sysclk = clk_get_rate(clk); - } else if (of_property_read_bool(np, "system-clock-frequency")) { - of_property_read_u32(np, - "system-clock-frequency", - &dai->sysclk); + } else if (!of_property_read_u32(np, "system-clock-frequency", &val)) { + dai->sysclk = val; } else { clk = of_clk_get(node, 0); if (!IS_ERR(clk)) @@ -303,6 +302,7 @@ static int asoc_simple_card_parse_of(struct device_node *node, { struct snd_soc_dai_link *dai_link = priv->snd_card.dai_link; struct simple_dai_props *dai_props = priv->dai_props; + u32 val; int ret; /* parsing the card name from DT */ @@ -325,8 +325,9 @@ static int asoc_simple_card_parse_of(struct device_node *node, } /* Factor to mclk, used in hw_params() */ - of_property_read_u32(node, "simple-audio-card,mclk-fs", - &priv->mclk_fs); + ret = of_property_read_u32(node, "simple-audio-card,mclk-fs", &val); + if (ret == 0) + priv->mclk_fs = val; dev_dbg(dev, "New simple-card: %s\n", priv->snd_card.name ? priv->snd_card.name : ""); From 25ccb22ed55cf4ed1b94e2627b80f9ef44637558 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Thu, 26 Jun 2014 08:06:55 +0300 Subject: [PATCH 2/5] ASoC: tlv320aic3x: Correct S24_3LE support Correct the hw_params callback to configure the codec correctly in case of S24_3LE format since in case of S24_3LE the codec has been configured to 16bit format mode. S24_LE is not defined as supported format for the codec. Signed-off-by: Peter Ujfalusi Signed-off-by: Mark Brown --- sound/soc/codecs/tlv320aic3x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index e12fafbb1e09..5360772bc1ad 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -879,7 +879,7 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, case SNDRV_PCM_FORMAT_S20_3LE: data |= (0x01 << 4); break; - case SNDRV_PCM_FORMAT_S24_LE: + case SNDRV_PCM_FORMAT_S24_3LE: data |= (0x02 << 4); break; case SNDRV_PCM_FORMAT_S32_LE: From d3d4e5247b013008a39e4d5f69ce4c60ed57f997 Mon Sep 17 00:00:00 2001 From: Sylwester Nawrocki Date: Fri, 4 Jul 2014 16:05:45 +0200 Subject: [PATCH 3/5] ASoC: samsung: Correct I2S DAI suspend/resume ops We should save/restore relevant I2S registers regardless of the dai->active flag, otherwise some settings are being lost after system suspend/resume cycle. E.g. I2S slave mode set only during dai initialization is not preserved and the device ends up in master mode after system resume. Signed-off-by: Sylwester Nawrocki Signed-off-by: Mark Brown Cc: stable@vger.kernel.org --- sound/soc/samsung/i2s.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c index 2ac76fa3e742..5f9b255a8b38 100644 --- a/sound/soc/samsung/i2s.c +++ b/sound/soc/samsung/i2s.c @@ -920,11 +920,9 @@ static int i2s_suspend(struct snd_soc_dai *dai) { struct i2s_dai *i2s = to_info(dai); - if (dai->active) { - i2s->suspend_i2smod = readl(i2s->addr + I2SMOD); - i2s->suspend_i2scon = readl(i2s->addr + I2SCON); - i2s->suspend_i2spsr = readl(i2s->addr + I2SPSR); - } + i2s->suspend_i2smod = readl(i2s->addr + I2SMOD); + i2s->suspend_i2scon = readl(i2s->addr + I2SCON); + i2s->suspend_i2spsr = readl(i2s->addr + I2SPSR); return 0; } @@ -933,11 +931,9 @@ static int i2s_resume(struct snd_soc_dai *dai) { struct i2s_dai *i2s = to_info(dai); - if (dai->active) { - writel(i2s->suspend_i2scon, i2s->addr + I2SCON); - writel(i2s->suspend_i2smod, i2s->addr + I2SMOD); - writel(i2s->suspend_i2spsr, i2s->addr + I2SPSR); - } + writel(i2s->suspend_i2scon, i2s->addr + I2SCON); + writel(i2s->suspend_i2smod, i2s->addr + I2SMOD); + writel(i2s->suspend_i2spsr, i2s->addr + I2SPSR); return 0; } From e42be7e1420389349400e990a3b13d23b9c60d39 Mon Sep 17 00:00:00 2001 From: Fabio Estevam Date: Mon, 7 Jul 2014 14:26:48 -0300 Subject: [PATCH 4/5] ASoC: sgtl5000: Fix driver unbound Using the sgtl5000 codec driver as a module and trying to remove it causes the followig kernel oops: root@freescale /$ rmmod snd-soc-imx-sgtl5000 [ 117.122920] ------------[ cut here ]------------ [ 117.127609] WARNING: CPU: 0 PID: 631 at drivers/regulator/core.c:3604 regula) [ 117.137046] Modules linked in: snd_soc_imx_sgtl5000(-) snd_soc_sgtl5000 evbug [ 117.144315] CPU: 0 PID: 631 Comm: rmmod Not tainted 3.16.0-rc3-next-201407043 [ 117.153366] Backtrace: [ 117.155865] [<80011e5c>] (dump_backtrace) from [<80011ff8>] (show_stack+0x18) [ 117.163484] r6:802fcc48 r5:00000000 r4:00000000 r3:00000000 [ 117.169228] [<80011fe0>] (show_stack) from [<80668cc0>] (dump_stack+0x88/0xa) [ 117.176508] [<80668c38>] (dump_stack) from [<80029a38>] (warn_slowpath_commo) [ 117.184696] r5:00000009 r4:00000000 [ 117.188322] [<800299c8>] (warn_slowpath_common) from [<80029a80>] (warn_slow) [ 117.197150] r8:dd60d600 r7:ddfa6d00 r6:dd5d9064 r5:dd5e0f90 r4:dd5d9400 [ 117.203983] [<80029a5c>] (warn_slowpath_null) from [<802fcc48>] (regulator_u) [ 117.212828] [<802fcb74>] (regulator_unregister) from [<7f0047c4>] (ldo_regul) [ 117.223475] r4:dd59e300 r3:dd5e0f90 [ 117.227100] [<7f00479c>] (ldo_regulator_remove [snd_soc_sgtl5000]) from [<7f) [ 117.238959] r4:dd5d8000 r3:ddd51420 [ 117.242623] [<7f0047dc>] (sgtl5000_remove [snd_soc_sgtl5000]) from [<804e5b1) [ 117.252489] r5:00000000 r4:dd5d8000 [ 117.256111] [<804e5af8>] (soc_remove_codec) from [<804e5ed4>] (soc_remove_da) [ 117.264933] r4:ddfb640c r3:00000000 [ 117.268555] [<804e5c10>] (soc_remove_dai_links) from [<804e5fbc>] (snd_soc_u) [ 117.277810] r10:80359e48 r9:dd684000 r8:dd5ca800 r7:dd685e60 r6:00000001 r5c [ 117.285761] r4:dd5d9064 [ 117.288329] [<804e5f28>] (snd_soc_unregister_card) from [<804f29f0>] (devm_c) [ 117.297324] r6:00000004 r5:ddd0fc10 r4:dd5a7980 r3:804f29dc [ 117.303095] [<804f29dc>] (devm_card_release) from [<8035a418>] (release_node) [ 117.311369] [<8035a2a8>] (release_nodes) from [<8035aab4>] (devres_release_a) [ 117.319583] r10:00000000 r9:dd684000 r8:8000ed64 r7:00000081 r6:ddd0fc44 r54 [ 117.327543] r4:ddd0fc10 [ 117.330108] [<8035aa7c>] (devres_release_all) from [<803571c8>] (__device_re) [ 117.339214] r4:ddd0fc10 r3:dd5d9010 [ 117.342989] [<80357148>] (__device_release_driver) from [<80357a48>] (driver) [ 117.351607] r5:7f00c9e4 r4:ddd0fc10 [ 117.355285] [<8035798c>] (driver_detach) from [<80357030>] (bus_remove_drive) [ 117.363454] r6:00000880 r5:00000000 r4:7f00c9e4 r3:dd6c3a80 [ 117.369195] [<80356fdc>] (bus_remove_driver) from [<803580b8>] (driver_unreg) [ 117.377683] r4:7f00c9e4 r3:dd60d480 [ 117.381305] [<80358088>] (driver_unregister) from [<80358bb4>] (platform_dri) [ 117.390642] r4:7f00ca28 r3:7f00c35c [ 117.394309] [<80358ba0>] (platform_driver_unregister) from [<7f00c370>] (imx) [ 117.406188] [<7f00c35c>] (imx_sgtl5000_driver_exit [snd_soc_imx_sgtl5000]) f) [ 117.417452] [<8008c2b8>] (SyS_delete_module) from [<8000eba0>] (ret_fast_sys) [ 117.425753] r6:5f636f73 r5:5f646e73 r4:00016f40 [ 117.430428] ---[ end trace 8fd8a5cb39e46d0e ]--- This problem is well explained by Russell King: "The sgtl5000 uses a two-stage initialisation process. The first stage is when the platform driver is probed, where some resources are found and initialised. The second stage is via the codec driver's probe function, where regulators are found and initialised using the managed resource support. The problem here is that this works fine until the codec driver is unbound. When this occurs, sgtl5000_remove() is called which is a no-op as far as the managed resource code is concerned. The regulators remain allocated, and their pointers in sgtl5000_priv remain valid. If the codec is now re-probed, it will again try and find the regulators, which will now be busy. This will fail. That's not the only problem - if using the LDO regulator, the regulator is unregistered from the regulator core code, but it still has a user. When the user is cleaned up (eg, by removing the module) it hits the free'd regulator, and this can oops the kernel. This bug was originally introduced by 63e54cd9caa3ce ("ASoC: sgtl5000: Use devm_regulator_bulk_get()")." This reverts commit 63e54cd9caa3ce. Tested on a imx53-qsb board. Reported-by: Russell King Signed-off-by: Fabio Estevam Signed-off-by: Mark Brown --- sound/soc/codecs/sgtl5000.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c index 3d39f0b5b4a8..8f4c73d17c87 100644 --- a/sound/soc/codecs/sgtl5000.c +++ b/sound/soc/codecs/sgtl5000.c @@ -1277,7 +1277,7 @@ static int sgtl5000_enable_regulators(struct snd_soc_codec *codec) return ret; } - ret = devm_regulator_bulk_get(codec->dev, ARRAY_SIZE(sgtl5000->supplies), + ret = regulator_bulk_get(codec->dev, ARRAY_SIZE(sgtl5000->supplies), sgtl5000->supplies); if (ret) goto err_ldo_remove; @@ -1285,13 +1285,16 @@ static int sgtl5000_enable_regulators(struct snd_soc_codec *codec) ret = regulator_bulk_enable(ARRAY_SIZE(sgtl5000->supplies), sgtl5000->supplies); if (ret) - goto err_ldo_remove; + goto err_regulator_free; /* wait for all power rails bring up */ udelay(10); return 0; +err_regulator_free: + regulator_bulk_free(ARRAY_SIZE(sgtl5000->supplies), + sgtl5000->supplies); err_ldo_remove: if (!external_vddd) ldo_regulator_remove(codec); @@ -1361,6 +1364,8 @@ static int sgtl5000_probe(struct snd_soc_codec *codec) err: regulator_bulk_disable(ARRAY_SIZE(sgtl5000->supplies), sgtl5000->supplies); + regulator_bulk_free(ARRAY_SIZE(sgtl5000->supplies), + sgtl5000->supplies); ldo_regulator_remove(codec); return ret; @@ -1374,6 +1379,8 @@ static int sgtl5000_remove(struct snd_soc_codec *codec) regulator_bulk_disable(ARRAY_SIZE(sgtl5000->supplies), sgtl5000->supplies); + regulator_bulk_free(ARRAY_SIZE(sgtl5000->supplies), + sgtl5000->supplies); ldo_regulator_remove(codec); return 0; From b97c60abf9a561f86ae71bd741add02673cc1a08 Mon Sep 17 00:00:00 2001 From: Sylwester Nawrocki Date: Thu, 10 Jul 2014 18:11:13 +0200 Subject: [PATCH 5/5] ASoC: samsung-i2s: Maintain CDCLK settings across i2s_{shutdown/startup} Currently configuration of the CDCLK pad is being overwritten in the i2s_shutdown() callback in order to gate the SoC output clock. However if an ASoC machine driver doesn't restore that clock settings each time after opening the sound device this results in the CDCLK pin being permanently configured into input mode. I.e. the output clock will always stay disabled. Fix that by saving the CDCLKCON bit state in i2s_shutdown() and and restoring it in the i2s_startup() callback. Signed-off-by: Chen Zhen Signed-off-by: Sylwester Nawrocki Signed-off-by: Mark Brown --- sound/soc/samsung/i2s.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c index 5f9b255a8b38..d2533dbc8399 100644 --- a/sound/soc/samsung/i2s.c +++ b/sound/soc/samsung/i2s.c @@ -68,6 +68,8 @@ struct i2s_dai { #define DAI_OPENED (1 << 0) /* Dai is opened */ #define DAI_MANAGER (1 << 1) /* Dai is the manager */ unsigned mode; + /* CDCLK pin direction: 0 - input, 1 - output */ + unsigned int cdclk_out:1; /* Driver for this DAI */ struct snd_soc_dai_driver i2s_dai_drv; /* DMA parameters */ @@ -737,6 +739,9 @@ static int i2s_startup(struct snd_pcm_substream *substream, spin_unlock_irqrestore(&lock, flags); + if (!is_opened(other) && i2s->cdclk_out) + i2s_set_sysclk(dai, SAMSUNG_I2S_CDCLK, + 0, SND_SOC_CLOCK_OUT); return 0; } @@ -752,9 +757,13 @@ static void i2s_shutdown(struct snd_pcm_substream *substream, i2s->mode &= ~DAI_OPENED; i2s->mode &= ~DAI_MANAGER; - if (is_opened(other)) + if (is_opened(other)) { other->mode |= DAI_MANAGER; - + } else { + u32 mod = readl(i2s->addr + I2SMOD); + i2s->cdclk_out = !(mod & MOD_CDCLKCON); + other->cdclk_out = i2s->cdclk_out; + } /* Reset any constraint on RFS and BFS */ i2s->rfs = 0; i2s->bfs = 0;