January 2019 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] [PATCH] ALSA: pcm: Use the common error path in __snd_pcm_lib_xfer()
by Takashi Iwai 25 Jan '19

25 Jan '19

An open-coded error path in __snd_pcm_lib_xfer() can be replaced with the simple goto to the common error path. This also makes the error handling more consistent, i.e. when some samples have been already processed, return that size instead of the error code. Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/core/pcm_lib.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 40013b26f671..f48efce937ad 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -2219,9 +2219,8 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, if (frames > cont) frames = cont; if (snd_BUG_ON(!frames)) { - runtime->twake = 0; - snd_pcm_stream_unlock_irq(substream); - return -EINVAL; + err = -EINVAL; + goto _end_unlock; } snd_pcm_stream_unlock_irq(substream); err = writer(substream, appl_ofs, data, offset, frames, -- 2.16.4

1 0

[alsa-devel] [PATCH] ALSA: pcm: Fix tight loop of OSS capture stream
by Takashi Iwai 25 Jan '19

25 Jan '19

When the trigger=off is passed for a PCM OSS stream, it sets the start_threshold of the substream to the boundary. This can be problematic for a capture stream, unfortunately, like the following, as detected by syzkaller: - In __snd_pcm_lib_xfer() that is invoked from snd_pcm_oss_read() loop, we have a check whether the stream was already started or the stream can be auto-started. - With trigger=0, we explicitly disable the auto-start, and the function returns 0 at this check. - The loop continues tightly, which may lead to an RCU stall. This patch fixes the bug by simply allowing the wait for non-started stream in the case of OSS capture. For native usages, it's supposed to be done by the caller side (which is user-space), hence it returns zero like before. (In theory, this could wait there, but I'd like to stay in a safer side for not breaking the existing stuff.) Reported-by: syzbot+fbe0496f92a0ce7b786c(a)syzkaller.appspotmail.com Cc: <stable(a)vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/core/pcm_lib.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 40013b26f671..6c99fa8ac5fa 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -2112,6 +2112,13 @@ int pcm_lib_apply_appl_ptr(struct snd_pcm_substream *substream, return 0; } +/* allow waiting for a capture stream that hasn't been started */ +#if IS_ENABLED(CONFIG_SND_PCM_OSS) +#define wait_capture_start(substream) ((substream)->oss.oss) +#else +#define wait_capture_start(substream) false +#endif + /* the common loop for read/write data */ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, void *data, bool interleaved, @@ -2182,7 +2189,7 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, err = snd_pcm_start(substream); if (err < 0) goto _end_unlock; - } else { + } else if (!wait_capture_start(substream)) { /* nothing to do */ err = 0; goto _end_unlock; -- 2.16.4

1 0

[alsa-devel] [PATCH] ALSA: fireface: Off by one in latter_handle_midi_msg()
by Dan Carpenter 25 Jan '19

25 Jan '19

The > should be >= or otherwise we potentially read one element beyond the end of the ff->tx_midi_substreams[] array. Fixes: 73f5537fb209 ("ALSA: fireface: support tx MIDI functionality of Fireface UCX") Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> --- sound/firewire/fireface/ff-protocol-latter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/firewire/fireface/ff-protocol-latter.c b/sound/firewire/fireface/ff-protocol-latter.c index 817af4447349..438aeed9a1ab 100644 --- a/sound/firewire/fireface/ff-protocol-latter.c +++ b/sound/firewire/fireface/ff-protocol-latter.c @@ -275,7 +275,7 @@ static void latter_handle_midi_msg(struct snd_ff *ff, unsigned int offset, struct snd_rawmidi_substream *substream; unsigned int len; - if (index > ff->spec->midi_in_ports) + if (index >= ff->spec->midi_in_ports) return; switch (data & 0x0000000f) { -- 2.17.1

3 2

[alsa-devel] [PATCH v2] ASoC: intel: cht_bsw_max98090_ti: Enable codec clock once and keep it enabled
by Hans de Goede 25 Jan '19

25 Jan '19

Users have been seeing sound stability issues with max98090 codecs since: commit 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL") At first that commit broke sound for Chromebook Swanky and Clapper models, the problem was that the machine-driver has been controlling the wrong clock on those models since support for them was added. This was hidden by clk-pmc-atom.c keeping the actual clk on unconditionally. With the machine-driver controlling the proper clock, sound works again but we are seeing bug reports describing it as: low volume, "sounds like played at 10x speed" and instable. When these issues are hit the following message is seen in dmesg: "max98090 i2c-193C9890:00: PLL unlocked". Attempts have been made to fix this by inserting a delay between enabling the clk and enabling and checking the pll, but this has not helped. It seems that at least on boards which use pmc_plt_clk_0 as clock, if we ever disable the clk, the pll looses its lock and after that we get various issues. This commit fixes this by enabling the clock once at probe time on these boards. In essence this restores the old behavior of clk-pmc-atom.c always keeping the clk on on these boards. Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL") Reported-by: Mogens Jensen <mogens-jensen(a)protonmail.com> Reported-by: Dean Wallace <duffydack73(a)gmail.com> Signed-off-by: Hans de Goede <hdegoede(a)redhat.com> --- Changes in v2: -Only enable the codec clock once and keep it enabled on boards which use pmc_plt_clk_0 as clock and leave the behavior unchanged on other boards --- sound/soc/intel/boards/cht_bsw_max98090_ti.c | 47 +++++++++++++++++--- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/sound/soc/intel/boards/cht_bsw_max98090_ti.c b/sound/soc/intel/boards/cht_bsw_max98090_ti.c index 08a5152e635a..e7620017e725 100644 --- a/sound/soc/intel/boards/cht_bsw_max98090_ti.c +++ b/sound/soc/intel/boards/cht_bsw_max98090_ti.c @@ -42,6 +42,7 @@ struct cht_mc_private { struct clk *mclk; struct snd_soc_jack jack; bool ts3a227e_present; + int quirks; }; static int platform_clock_control(struct snd_soc_dapm_widget *w, @@ -53,6 +54,10 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w, struct cht_mc_private *ctx = snd_soc_card_get_drvdata(card); int ret; + /* See the comment in snd_cht_mc_probe() */ + if (ctx->quirks & QUIRK_PMC_PLT_CLK_0) + return 0; + codec_dai = snd_soc_card_get_codec_dai(card, CHT_CODEC_DAI); if (!codec_dai) { dev_err(card->dev, "Codec dai not found; Unable to set platform clock\n"); @@ -222,6 +227,10 @@ static int cht_codec_init(struct snd_soc_pcm_runtime *runtime) "jack detection gpios not added, error %d\n", ret); } + /* See the comment in snd_cht_mc_probe() */ + if (ctx->quirks & QUIRK_PMC_PLT_CLK_0) + return 0; + /* * The firmware might enable the clock at * boot (this information may or may not @@ -420,16 +429,15 @@ static int snd_cht_mc_probe(struct platform_device *pdev) int ret_val = 0; struct cht_mc_private *drv; const char *mclk_name; - int quirks = 0; - - dmi_id = dmi_first_match(cht_max98090_quirk_table); - if (dmi_id) - quirks = (unsigned long)dmi_id->driver_data; drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL); if (!drv) return -ENOMEM; + dmi_id = dmi_first_match(cht_max98090_quirk_table); + if (dmi_id) + drv->quirks = (unsigned long)dmi_id->driver_data; + drv->ts3a227e_present = acpi_dev_found("104C227E"); if (!drv->ts3a227e_present) { /* no need probe TI jack detection chip */ @@ -446,7 +454,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev) snd_soc_card_cht.dev = &pdev->dev; snd_soc_card_set_drvdata(&snd_soc_card_cht, drv); - if (quirks & QUIRK_PMC_PLT_CLK_0) + if (drv->quirks & QUIRK_PMC_PLT_CLK_0) mclk_name = "pmc_plt_clk_0"; else mclk_name = "pmc_plt_clk_3"; @@ -459,6 +467,21 @@ static int snd_cht_mc_probe(struct platform_device *pdev) return PTR_ERR(drv->mclk); } + /* + * Boards which have the MAX98090's clk connected to clk_0 do not seem + * to like it if we muck with the clock. If we disable the clock when + * it is unused we get "max98090 i2c-193C9890:00: PLL unlocked" errors + * and the PLL never seems to lock again. + * So for these boards we enable it here once and leave it at that. + */ + if (drv->quirks & QUIRK_PMC_PLT_CLK_0) { + ret_val = clk_prepare_enable(drv->mclk); + if (ret_val < 0) { + dev_err(&pdev->dev, "MCLK enable error: %d\n", ret_val); + return ret_val; + } + } + ret_val = devm_snd_soc_register_card(&pdev->dev, &snd_soc_card_cht); if (ret_val) { dev_err(&pdev->dev, @@ -469,11 +492,23 @@ static int snd_cht_mc_probe(struct platform_device *pdev) return ret_val; } +static int snd_cht_mc_remove(struct platform_device *pdev) +{ + struct snd_soc_card *card = platform_get_drvdata(pdev); + struct cht_mc_private *ctx = snd_soc_card_get_drvdata(card); + + if (ctx->quirks & QUIRK_PMC_PLT_CLK_0) + clk_disable_unprepare(ctx->mclk); + + return 0; +} + static struct platform_driver snd_cht_mc_driver = { .driver = { .name = "cht-bsw-max98090", }, .probe = snd_cht_mc_probe, + .remove = snd_cht_mc_remove, }; module_platform_driver(snd_cht_mc_driver) -- 2.20.1

1 0

[alsa-devel] [PATCH] ASoC: soc-core: Fix null pointer dereference in soc_find_component
by Rohit kumar 25 Jan '19

25 Jan '19

From: Ajit Pandey <ajitp(a)codeaurora.org> soc_find_component() may lead to null pointer exception if both arguments i.e of_node and name is NULL. Add NULL check before calling soc_find_component(). Also fix some typos. Fixes: 8780cf1142a5 ("ASoC: soc-core: defer card probe until all component is added to list") Reported-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Signed-off-by: Ajit Pandey <ajitp(a)codeaurora.org> Signed-off-by: Rohit kumar <rohitkr(a)codeaurora.org> --- sound/soc/soc-core.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 0934b36..df05fb8 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1131,11 +1131,13 @@ static int soc_init_dai_link(struct snd_soc_card *card, } /* - * Defer card registartion if platform dai component is not added to + * Defer card registration if platform dai component is not added to * component list. */ - if (!soc_find_component(link->platform->of_node, link->platform->name)) - return -EPROBE_DEFER; + if (link->platform->of_node || link->platform->name) + if (!soc_find_component(link->platform->of_node, + link->platform->name)) + return -EPROBE_DEFER; /* * CPU device may be specified by either name or OF node, but @@ -1150,11 +1152,12 @@ static int soc_init_dai_link(struct snd_soc_card *card, } /* - * Defer card registartion if cpu dai component is not added to + * Defer card registration if cpu dai component is not added to * component list. */ - if (!soc_find_component(link->cpu_of_node, link->cpu_name)) - return -EPROBE_DEFER; + if (link->cpu_of_node || link->cpu_name) + if (!soc_find_component(link->cpu_of_node, link->cpu_name)) + return -EPROBE_DEFER; /* * At least one of CPU DAI name or CPU device name/node must be -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

6 25

[alsa-devel] [PATCH][V2] ASoC: Intel: make const arrays static, reduces object code size
by Colin King 24 Jan '19

24 Jan '19

From: Colin Ian King <colin.king(a)canonical.com> Don't populate the const arrays on the stack but instead make it static. Makes the object code smaller, for example: Before: text data bss dec hex filename 14107 8832 224 23163 5a7b bytcht_es8316.o After: text data bss dec hex filename 14015 8896 224 23135 5a5f bytcht_es8316.o (gcc version 8.2.0 x86_64) Signed-off-by: Colin Ian King <colin.king(a)canonical.com> --- V2: also fix up bytcr_rt5640.c and bytcr_rt5651.c, thanks to Pierre-Louis Bossart for noticing these two. --- sound/soc/intel/boards/bytcht_es8316.c | 2 +- sound/soc/intel/boards/bytcr_rt5640.c | 2 +- sound/soc/intel/boards/bytcr_rt5651.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c index fa9c4cf97686..1364e4e601d8 100644 --- a/sound/soc/intel/boards/bytcht_es8316.c +++ b/sound/soc/intel/boards/bytcht_es8316.c @@ -437,7 +437,7 @@ static const struct acpi_gpio_mapping byt_cht_es8316_gpios[] = { static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev) { - const char * const mic_name[] = { "in1", "in2" }; + static const char * const mic_name[] = { "in1", "in2" }; struct byt_cht_es8316_private *priv; struct device *dev = &pdev->dev; struct snd_soc_acpi_mach *mach; diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index ca8b4d5ff70f..a79466c8fb29 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -1149,7 +1149,7 @@ struct acpi_chan_package { /* ACPICA seems to require 64 bit integers */ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) { - const char * const map_name[] = { "dmic1", "dmic2", "in1", "in3" }; + static const char * const map_name[] = { "dmic1", "dmic2", "in1", "in3" }; const struct dmi_system_id *dmi_id; struct byt_rt5640_private *priv; struct snd_soc_acpi_mach *mach; diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c index b618d984e2d5..e6945d11c8ab 100644 --- a/sound/soc/intel/boards/bytcr_rt5651.c +++ b/sound/soc/intel/boards/bytcr_rt5651.c @@ -919,7 +919,7 @@ struct acpi_chan_package { /* ACPICA seems to require 64 bit integers */ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev) { - const char * const mic_name[] = { "dmic", "in1", "in2", "in12" }; + static const char * const mic_name[] = { "dmic", "in1", "in2", "in12" }; struct byt_rt5651_private *priv; struct snd_soc_acpi_mach *mach; struct device *codec_dev; -- 2.19.1

3 2

[alsa-devel] [PATCH] ALSA: hda: runtime PM is always active
by Sameer Pujar 24 Jan '19

24 Jan '19

The runtime PM count is incremented and set to active during hda codec device init, but it is decremented and set to suspend during exit only. Hence the runtime PM status is always active and hda device cannot be put to runtime suspend. Keeping device usage active for entire period, though nothing really happening on the device, seems unnecessary. This patch avoides incrementing runtime PM usage count of the device. Signed-off-by: Sameer Pujar <spujar(a)nvidia.com> Reviewed-by: Ravindra Lokhande <rlokhande(a)nvidia.com> Reviewed-by: Mohan Kumar D <mkumard(a)nvidia.com> --- sound/hda/hdac_device.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c index 95b073e..72aa342 100644 --- a/sound/hda/hdac_device.c +++ b/sound/hda/hdac_device.c @@ -31,9 +31,6 @@ static void default_release(struct device *dev) * * Returns zero for success or a negative error code. * - * This function increments the runtime PM counter and marks it active. - * The caller needs to turn it off appropriately later. - * * The caller needs to set the device's release op properly by itself. */ int snd_hdac_device_init(struct hdac_device *codec, struct hdac_bus *bus, @@ -55,8 +52,6 @@ int snd_hdac_device_init(struct hdac_device *codec, struct hdac_bus *bus, codec->bus = bus; codec->addr = addr; codec->type = HDA_DEV_CORE; - pm_runtime_set_active(&codec->dev); - pm_runtime_get_noresume(&codec->dev); atomic_set(&codec->in_pm, 0); err = snd_hdac_bus_add_device(bus, codec); @@ -123,7 +118,6 @@ EXPORT_SYMBOL_GPL(snd_hdac_device_init); */ void snd_hdac_device_exit(struct hdac_device *codec) { - pm_runtime_put_noidle(&codec->dev); snd_hdac_bus_remove_device(codec->bus, codec); kfree(codec->vendor_name); kfree(codec->chip_name); -- 2.7.4

2 5

[alsa-devel] [RFC PATCH] ASoC: pcm512x: Implement the set_bclk_ratio interface
by Dimitris Papavasiliou 24 Jan '19

24 Jan '19

>From 5e6ad135fd063d4b22cd962de43499a161bbc7db Mon Sep 17 00:00:00 2001 From: Dimitris Papavasiliou <dpapavas(a)gmail.com> Date: Fri, 11 Jan 2019 22:13:27 +0200 Subject: [RFC PATCH] ASoC: pcm512x: Implement the set_bclk_ratio interface Some boards, such as the HiFiBerry DAC+ Pro, use a pair of external oscillators, to generate 44.1 or 48kHz multiples and are forced to resort to hacks[1] in order to support 24-bit data without ending up with fractional dividers. This patch allows the machine driver to use 32-bit frames for 24-bit data to avoid such issues. [1] http://mailman.alsa-project.org/pipermail/alsa-devel/2018-December/143442.h… Signed-off-by: Dimitris Papavasiliou <dpapavas(a)gmail.com> --- Notes: The patch also includes a related change in the calculation of the bclk divider. The current calculation is not very clear to me, as I can't find much concrete information on what this sample rate fraction is, how it's calculated and how, therefore, its denominator would fit into the calculation. It also yields slightly wrong dividers in certain cases, which the machine driver for my HiFiBerry DAC+ Pro board seemingly tries to circumvent, by updating the rate fraction so as to suit this calculation. The updated calculation seems to work fine in my tests and, as far as I can see, should correctly yield the smallest bit clock rate that would fit the frame. Please comment if anything looks off. sound/soc/codecs/pcm512x.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/sound/soc/codecs/pcm512x.c b/sound/soc/codecs/pcm512x.c index 4cc24a5d5c31..8cd728f9a1eb 100644 --- a/sound/soc/codecs/pcm512x.c +++ b/sound/soc/codecs/pcm512x.c @@ -55,6 +55,7 @@ struct pcm512x_priv { unsigned long overclock_dsp; int mute; struct mutex mutex; + unsigned int bclk_ratio; }; /* @@ -915,16 +916,21 @@ static int pcm512x_set_dividers(struct snd_soc_dai *dai, int fssp; int gpio; - lrclk_div = snd_soc_params_to_frame_size(params); - if (lrclk_div == 0) { - dev_err(dev, "No LRCLK?\n"); - return -EINVAL; + if (pcm512x->bclk_ratio > 0) { + lrclk_div = pcm512x->bclk_ratio; + } else { + lrclk_div = snd_soc_params_to_frame_size(params); + + if (lrclk_div == 0) { + dev_err(dev, "No LRCLK?\n"); + return -EINVAL; + } } if (!pcm512x->pll_out) { sck_rate = clk_get_rate(pcm512x->sclk); - bclk_div = params->rate_den * 64 / lrclk_div; - bclk_rate = DIV_ROUND_CLOSEST(sck_rate, bclk_div); + bclk_rate = params_rate(params) * lrclk_div; + bclk_div = DIV_ROUND_CLOSEST(sck_rate, bclk_rate); mck_rate = sck_rate; } else { @@ -1383,6 +1389,16 @@ static int pcm512x_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) return 0; } +static int pcm512x_set_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio) +{ + struct snd_soc_component *component = dai->component; + struct pcm512x_priv *pcm512x = snd_soc_component_get_drvdata(component); + + pcm512x->bclk_ratio = ratio; + + return 0; +} + static int pcm512x_digital_mute(struct snd_soc_dai *dai, int mute) { struct snd_soc_component *component = dai->component; @@ -1435,6 +1451,7 @@ static const struct snd_soc_dai_ops pcm512x_dai_ops = { .hw_params = pcm512x_hw_params, .set_fmt = pcm512x_set_fmt, .digital_mute = pcm512x_digital_mute, + .set_bclk_ratio = pcm512x_set_bclk_ratio, }; static struct snd_soc_dai_driver pcm512x_dai = { -- 2.11.0

4 10

[alsa-devel] [PATCH][next] ASoC: Intel: bytcht_es8316: make mic_name static, reduces object code size
by Colin King 24 Jan '19

24 Jan '19

From: Colin Ian King <colin.king(a)canonical.com> Don't populate the const array mic_name on the stack but instead make it static. Makes the object code smaller by 28 bytes: Before: text data bss dec hex filename 14107 8832 224 23163 5a7b bytcht_es8316.o After: text data bss dec hex filename 14015 8896 224 23135 5a5f bytcht_es8316.o (gcc version 8.2.0 x86_64) Signed-off-by: Colin Ian King <colin.king(a)canonical.com> --- sound/soc/intel/boards/bytcht_es8316.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c index fa9c4cf97686..1364e4e601d8 100644 --- a/sound/soc/intel/boards/bytcht_es8316.c +++ b/sound/soc/intel/boards/bytcht_es8316.c @@ -437,7 +437,7 @@ static const struct acpi_gpio_mapping byt_cht_es8316_gpios[] = { static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev) { - const char * const mic_name[] = { "in1", "in2" }; + static const char * const mic_name[] = { "in1", "in2" }; struct byt_cht_es8316_private *priv; struct device *dev = &pdev->dev; struct snd_soc_acpi_mach *mach; -- 2.19.1

3 2

[alsa-devel] [PATCH] ASoC: intel: cht_bsw_max98090_ti: Enable codec clock once and keep it enabled
by Hans de Goede 24 Jan '19

24 Jan '19

Users have been seeing sound stability issues with max98090 codecs since: commit 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL") At first that commit broke sound for Chromebook Swanky and Clapper models, the problem was that the machine-driver has been controlling the wrong clock on those models since support for them was added. This was hidden by clk-pmc-atom.c keeping the actual clk on unconditionally. With the machine-driver controlling the proper clock, sound works again but we are seeing bug reports describing it as: low volume, "sounds like played at 10x speed" and instable. When these issues are hit the following message is seen in dmesg: "max98090 i2c-193C9890:00: PLL unlocked". Attempts have been made to fix this by inserting a delay between enabling the clk and enabling and checking the pll, but this has not helped. It seems that if we ever disable the clk, the pll looses its lock and then we get various issues. This commit fixes this by enabling the clock once at probe time, in essence restoring the old behavior of clk-pmc-atom.c always keeping the clk on, but then only on machines with a max98090 codec. Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL") Reported-and-tested-by: Mogens Jensen <mogens-jensen(a)protonmail.com> Reported-and-tested-by: Dean Wallace <duffydack73(a)gmail.com> Signed-off-by: Hans de Goede <hdegoede(a)redhat.com> --- sound/soc/intel/boards/cht_bsw_max98090_ti.c | 75 ++++++-------------- 1 file changed, 20 insertions(+), 55 deletions(-) diff --git a/sound/soc/intel/boards/cht_bsw_max98090_ti.c b/sound/soc/intel/boards/cht_bsw_max98090_ti.c index 08a5152e635a..549d6ce12ec4 100644 --- a/sound/soc/intel/boards/cht_bsw_max98090_ti.c +++ b/sound/soc/intel/boards/cht_bsw_max98090_ti.c @@ -44,43 +44,11 @@ struct cht_mc_private { bool ts3a227e_present; }; -static int platform_clock_control(struct snd_soc_dapm_widget *w, - struct snd_kcontrol *k, int event) -{ - struct snd_soc_dapm_context *dapm = w->dapm; - struct snd_soc_card *card = dapm->card; - struct snd_soc_dai *codec_dai; - struct cht_mc_private *ctx = snd_soc_card_get_drvdata(card); - int ret; - - codec_dai = snd_soc_card_get_codec_dai(card, CHT_CODEC_DAI); - if (!codec_dai) { - dev_err(card->dev, "Codec dai not found; Unable to set platform clock\n"); - return -EIO; - } - - if (SND_SOC_DAPM_EVENT_ON(event)) { - ret = clk_prepare_enable(ctx->mclk); - if (ret < 0) { - dev_err(card->dev, - "could not configure MCLK state"); - return ret; - } - } else { - clk_disable_unprepare(ctx->mclk); - } - - return 0; -} - static const struct snd_soc_dapm_widget cht_dapm_widgets[] = { SND_SOC_DAPM_HP("Headphone", NULL), SND_SOC_DAPM_MIC("Headset Mic", NULL), SND_SOC_DAPM_MIC("Int Mic", NULL), SND_SOC_DAPM_SPK("Ext Spk", NULL), - SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0, - platform_clock_control, SND_SOC_DAPM_PRE_PMU | - SND_SOC_DAPM_POST_PMD), }; static const struct snd_soc_dapm_route cht_audio_map[] = { @@ -97,10 +65,6 @@ static const struct snd_soc_dapm_route cht_audio_map[] = { {"codec_in0", NULL, "ssp2 Rx" }, {"codec_in1", NULL, "ssp2 Rx" }, {"ssp2 Rx", NULL, "HiFi Capture"}, - {"Headphone", NULL, "Platform Clock"}, - {"Headset Mic", NULL, "Platform Clock"}, - {"Int Mic", NULL, "Platform Clock"}, - {"Ext Spk", NULL, "Platform Clock"}, }; static const struct snd_kcontrol_new cht_mc_controls[] = { @@ -222,25 +186,6 @@ static int cht_codec_init(struct snd_soc_pcm_runtime *runtime) "jack detection gpios not added, error %d\n", ret); } - /* - * The firmware might enable the clock at - * boot (this information may or may not - * be reflected in the enable clock register). - * To change the rate we must disable the clock - * first to cover these cases. Due to common - * clock framework restrictions that do not allow - * to disable a clock that has not been enabled, - * we need to enable the clock first. - */ - ret = clk_prepare_enable(ctx->mclk); - if (!ret) - clk_disable_unprepare(ctx->mclk); - - ret = clk_set_rate(ctx->mclk, CHT_PLAT_CLK_3_HZ); - - if (ret) - dev_err(runtime->dev, "unable to set MCLK rate\n"); - return ret; } @@ -459,6 +404,16 @@ static int snd_cht_mc_probe(struct platform_device *pdev) return PTR_ERR(drv->mclk); } + /* + * The MAX98090 does not seem to like it if we muck with its clock, + * so we enable it here once and leave it at that. + */ + ret_val = clk_prepare_enable(drv->mclk); + if (ret_val < 0) { + dev_err(&pdev->dev, "Failed to enable MCLK: %d\n", ret_val); + return ret_val; + } + ret_val = devm_snd_soc_register_card(&pdev->dev, &snd_soc_card_cht); if (ret_val) { dev_err(&pdev->dev, @@ -469,11 +424,21 @@ static int snd_cht_mc_probe(struct platform_device *pdev) return ret_val; } +static int snd_cht_mc_remove(struct platform_device *pdev) +{ + struct snd_soc_card *card = platform_get_drvdata(pdev); + struct cht_mc_private *ctx = snd_soc_card_get_drvdata(card); + + clk_disable_unprepare(ctx->mclk); + return 0; +} + static struct platform_driver snd_cht_mc_driver = { .driver = { .name = "cht-bsw-max98090", }, .probe = snd_cht_mc_probe, + .remove = snd_cht_mc_remove, }; module_platform_driver(snd_cht_mc_driver) -- 2.20.1

2 3