May 2013 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] [PATCH]ALSA: HDA: Fix Oops caused by dereference NULL pointer
by Wang YanQing 07 May '13

07 May '13

The interrupt handler azx_interrupt will call azx_update_rirb, which may call snd_hda_queue_unsol_event, snd_hda_queue_unsol_event will dereference chip->bus pointer. The problem is we alloc chip->bus in azx_codec_create which will be called after we enable IRQ and enable unsolicited event in azx_probe. This will cause Oops due dereference NULL pointer. I meet it, good luck:) Signed-off-by: Wang YanQing <udknight(a)gmail.com> --- sound/pci/hda/hda_codec.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 622f726..9c76752 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -618,6 +618,9 @@ int snd_hda_queue_unsol_event(struct hda_bus *bus, u32 res, u32 res_ex) unsigned int wp; trace_hda_unsol_event(bus, res, res_ex); + if (!bus) + return 0; + unsol = bus->unsol; if (!unsol) return 0; -- 1.7.12.4.dirty

2 1

[alsa-devel] [PATCH 1/6] ASoC: Samsung: Remove redundant platform_set_drvdata()
by Sachin Kamat 07 May '13

07 May '13

Commit 0998d06310 (device-core: Ensure drvdata = NULL when no driver is bound) removes the need to set driver data field to NULL. Signed-off-by: Sachin Kamat <sachin.kamat(a)linaro.org> Cc: Sangbeom Kim <sbkim73(a)samsung.com> --- sound/soc/samsung/smdk_wm8580pcm.c | 1 - sound/soc/samsung/smdk_wm8994pcm.c | 1 - 2 files changed, 2 deletions(-) diff --git a/sound/soc/samsung/smdk_wm8580pcm.c b/sound/soc/samsung/smdk_wm8580pcm.c index e43bd42..23a9204 100644 --- a/sound/soc/samsung/smdk_wm8580pcm.c +++ b/sound/soc/samsung/smdk_wm8580pcm.c @@ -176,7 +176,6 @@ static int snd_smdk_probe(struct platform_device *pdev) static int snd_smdk_remove(struct platform_device *pdev) { snd_soc_unregister_card(&smdk_pcm); - platform_set_drvdata(pdev, NULL); return 0; } diff --git a/sound/soc/samsung/smdk_wm8994pcm.c b/sound/soc/samsung/smdk_wm8994pcm.c index 3688a32..0c84ca0 100644 --- a/sound/soc/samsung/smdk_wm8994pcm.c +++ b/sound/soc/samsung/smdk_wm8994pcm.c @@ -146,7 +146,6 @@ static int snd_smdk_probe(struct platform_device *pdev) static int snd_smdk_remove(struct platform_device *pdev) { snd_soc_unregister_card(&smdk_pcm); - platform_set_drvdata(pdev, NULL); return 0; } -- 1.7.9.5

4 12

[alsa-devel] [PATCH v2 12/13] ASoC: fsl: imx-audmux: Let device core handle pinctrl
by Fabio Estevam 07 May '13

07 May '13

From: Fabio Estevam <fabio.estevam(a)freescale.com> Since commit ab78029 (drivers/pinctrl: grab default handles from device core), we can rely on device core for handling pinctrl. So remove devm_pinctrl_get_select_default() from the driver. Signed-off-by: Fabio Estevam <fabio.estevam(a)freescale.com> --- Changes since v1: - Rebased against topic/fsl branch sound/soc/fsl/imx-audmux.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/sound/soc/fsl/imx-audmux.c b/sound/soc/fsl/imx-audmux.c index 47f046a..e260f1f 100644 --- a/sound/soc/fsl/imx-audmux.c +++ b/sound/soc/fsl/imx-audmux.c @@ -26,7 +26,6 @@ #include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/slab.h> -#include <linux/pinctrl/consumer.h> #include "imx-audmux.h" @@ -247,7 +246,6 @@ EXPORT_SYMBOL_GPL(imx_audmux_v2_configure_port); static int imx_audmux_probe(struct platform_device *pdev) { struct resource *res; - struct pinctrl *pinctrl; const struct of_device_id *of_id = of_match_device(imx_audmux_dt_ids, &pdev->dev); @@ -256,12 +254,6 @@ static int imx_audmux_probe(struct platform_device *pdev) if (IS_ERR(audmux_base)) return PTR_ERR(audmux_base); - pinctrl = devm_pinctrl_get_select_default(&pdev->dev); - if (IS_ERR(pinctrl)) { - dev_err(&pdev->dev, "setup pinctrl failed!"); - return PTR_ERR(pinctrl); - } - audmux_clk = devm_clk_get(&pdev->dev, "audmux"); if (IS_ERR(audmux_clk)) { dev_dbg(&pdev->dev, "cannot get clock: %ld\n", -- 1.7.9.5

2 1

Re: [alsa-devel] [RFC] ASoC: sgtl5000: Remove cache support
by Mark Brown 07 May '13

07 May '13

On Mon, May 06, 2013 at 03:19:27PM -0500, Matt Sealey wrote: > I have asked before and I will ask again; does anyone have any insight Not on the list you haven't as far as I can tell... this is the first I've heard of any concern about this code, I'd not be surprised if others were similarly unaware of any concerns. > The patch came from out of nowhere, and the comments seem to say that > the LDO gets enabled on every chip higher than a specific version. > Now, this is totally against the design documentation for the SGTL5000 > which specifically states that using the internal LDO is quite > possibly the *worst* power consuming case and NOT best practise for > putting the codec on a design, with the sole benefit being reduced > external component count. Are you sure it's purely about power consumption? A common application for LDOs in this situation is to clean up noise on the incoming power supply, improving rejection of noise. This is mostly irrelevant for digital supplies but can be desirable for analogue ones. Of course this looks like it's being used as a digital supply so... I have to say your explanation is a little curious, though - normally an LDO would require an external capacitor and so would either increase the component count (adding the capactior) or have no effect (if one is already present for decoupling). What is the supply for this LDO and where does the output go? > What happens if we just take out the internal LDO setup conditional on > version, and only initialize it if VDDD is missing? Do boards that If the LDO is enabled and connected to a supply which is also driven externally that's not a clever idea, the two regulators will both be trying to regulate the same supply. > I am not entirely sure how well the commit linked by Fabio would work > on mainline since if it uses hw_read/write it should also be updating > the cache with those values to be sure they can later be modified... > the BSP code does that then fills the cache with the existing values > afterwards. That commit is just bogus in the way it interacts with the cache, half of it looks awfully like it's trying to work around some breakage in the way the cache was being set up. > So maybe we don't disable the cache, or it gets ported to regmap, but > the order should be enable regulators in a non-cached way then fill > the cache AFTERWARDS - the code at ToT mainline doesn't fix the cache I'm sorry, this makes no sense to me. If we know the physical defaults for the device and we can get the device into that state then we should have no need to read from the cache. If we don't know what they are then they shouldn't be listed. > after it does regulator/power regs setup, I assume because cache > filling is now somehow automated in ASoC (and if this is the case, There is no cache filling to automate, if (as is the case here) register defaults are provided they're used. > then the cache io setup needs to be done after regulator/power setup, > the defaults list/cache needs to be updated for those registers before > any register caching io is done..). And maybe setting up the regulator > is kind of pointless anyway (meaning that whole caching/not-caching > thing just falls away?) Again this makes no sense, it seems like you're very confused here. The whole purpose of the register defaults is to reflect the default values of the registers. The infrastructure relies on this being the case, otherwise it will get confused for example during cache sync when it decides if a write to the device is required. I think the point that you lost me was your assumption that the regulator enable needs to be done without the cache. What makes you believe that this is the case? Note that as I said in another post the driver is currently totally broken with regard to anything that actually powers off the device, there's never any attempt made to restore the register cache.

1 0

[alsa-devel] [PATCH 13/13] ASoC: mxs: mxs-saif: Let device core handle pinctrl
by Fabio Estevam 06 May '13

06 May '13

Since commit ab78029 (drivers/pinctrl: grab default handles from device core), we can rely on device core for handling pinctrl. So remove devm_pinctrl_get_select_default() from the driver. Cc: Mark Brown <broonie(a)kernel.org> Cc: <alsa-devel(a)alsa-project.org> Signed-off-by: Fabio Estevam <fabio.estevam(a)freescale.com> --- sound/soc/mxs/mxs-saif.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c index 3a2aa1d..12723b0 100644 --- a/sound/soc/mxs/mxs-saif.c +++ b/sound/soc/mxs/mxs-saif.c @@ -27,7 +27,6 @@ #include <linux/delay.h> #include <linux/time.h> #include <linux/fsl/mxs-dma.h> -#include <linux/pinctrl/consumer.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> @@ -661,7 +660,6 @@ static int mxs_saif_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node; struct resource *iores, *dmares; struct mxs_saif *saif; - struct pinctrl *pinctrl; int ret = 0; struct device_node *master; @@ -701,12 +699,6 @@ static int mxs_saif_probe(struct platform_device *pdev) mxs_saif[saif->id] = saif; - pinctrl = devm_pinctrl_get_select_default(&pdev->dev); - if (IS_ERR(pinctrl)) { - ret = PTR_ERR(pinctrl); - return ret; - } - saif->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(saif->clk)) { ret = PTR_ERR(saif->clk); -- 1.7.9.5

2 1

[alsa-devel] [PATCH 12/13] ASoC: fsl: imx-audmux: Let device core handle pinctrl
by Fabio Estevam 06 May '13

06 May '13

Since commit ab78029 (drivers/pinctrl: grab default handles from device core), we can rely on device core for handling pinctrl. So remove devm_pinctrl_get_select_default() from the driver. Cc: Mark Brown <broonie(a)kernel.org> Cc: <alsa-devel(a)alsa-project.org> Signed-off-by: Fabio Estevam <fabio.estevam(a)freescale.com> --- sound/soc/fsl/imx-audmux.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/sound/soc/fsl/imx-audmux.c b/sound/soc/fsl/imx-audmux.c index 3f333e5..22967c6 100644 --- a/sound/soc/fsl/imx-audmux.c +++ b/sound/soc/fsl/imx-audmux.c @@ -26,7 +26,6 @@ #include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/slab.h> -#include <linux/pinctrl/consumer.h> #include "imx-audmux.h" @@ -247,7 +246,6 @@ EXPORT_SYMBOL_GPL(imx_audmux_v2_configure_port); static int imx_audmux_probe(struct platform_device *pdev) { struct resource *res; - struct pinctrl *pinctrl; const struct of_device_id *of_id = of_match_device(imx_audmux_dt_ids, &pdev->dev); @@ -256,12 +254,6 @@ static int imx_audmux_probe(struct platform_device *pdev) if (IS_ERR(audmux_base)) return PTR_ERR(audmux_base); - pinctrl = devm_pinctrl_get_select_default(&pdev->dev); - if (IS_ERR(pinctrl)) { - dev_err(&pdev->dev, "setup pinctrl failed!"); - return PTR_ERR(pinctrl); - } - audmux_clk = clk_get(&pdev->dev, "audmux"); if (IS_ERR(audmux_clk)) { dev_dbg(&pdev->dev, "cannot get clock: %ld\n", -- 1.7.9.5

2 1

[alsa-devel] [PATCH 1/2] ASoC: mxs-sgtl5000: Remove unneeded 'ret' variable
by Fabio Estevam 06 May '13

06 May '13

From: Fabio Estevam <fabio.estevam(a)freescale.com> Variable 'ret' is not needed here, so just remove it. Signed-off-by: Fabio Estevam <fabio.estevam(a)freescale.com> --- sound/soc/mxs/mxs-sgtl5000.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c index b1d9b5e..4f74b05 100644 --- a/sound/soc/mxs/mxs-sgtl5000.c +++ b/sound/soc/mxs/mxs-sgtl5000.c @@ -116,7 +116,7 @@ static int mxs_sgtl5000_probe_dt(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; struct device_node *saif_np[2], *codec_np; - int i, ret = 0; + int i; if (!np) return 1; /* no device tree */ @@ -142,7 +142,7 @@ static int mxs_sgtl5000_probe_dt(struct platform_device *pdev) of_node_put(saif_np[0]); of_node_put(saif_np[1]); - return ret; + return 0; } static int mxs_sgtl5000_probe(struct platform_device *pdev) -- 1.7.9.5

3 5

[alsa-devel] [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume
by mengdong.lin＠intel.com 06 May '13

06 May '13

From: Mengdong Lin <mengdong.lin(a)intel.com> In system resume, Haswell codec cannot be programmed to D0 before Gfx driver initializes the display pipeline and audio, which will trigger an unsol event on the pin with HDMI/DP cable connected. Otherwise, the connected pin will stay in D3 with right channel muted and thus no sound can be heard. This patch - adds a codec flag to delay resuming a codec. System resume will skip the codecs if this flag is set, and these codecs will be resumed on later codec access. - adds a set_power_state ops for Haswell HDMI codec. In a delayed resume, this ops will enable and wait for the unsol event, and then resume the codec. A 300ms timeout is set in case unsol event is lost. Signed-off-by: Mengdong Lin <mengdong.lin(a)intel.com> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 04b5738..bcb7205 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -5508,11 +5508,15 @@ int snd_hda_resume(struct hda_bus *bus) struct hda_codec *codec; list_for_each_entry(codec, &bus->codec_list, list) { - hda_call_codec_resume(codec); + if (codec->support_delay_resume) + codec->resume_delayed = 1; + else + hda_call_codec_resume(codec); } return 0; } EXPORT_SYMBOL_HDA(snd_hda_resume); + #endif /* CONFIG_PM */ /* diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index 23ca172..5b5e5f4 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -886,6 +886,8 @@ struct hda_codec { unsigned int d3_stop_clk:1; /* support D3 operation without BCLK */ unsigned int pm_down_notified:1; /* PM notified to controller */ unsigned int in_pm:1; /* suspend/resume being performed */ + unsigned int support_delay_resume:1; /* codec support delay resume */ + unsigned int resume_delayed:1; /* resume delayed by PM */ int power_transition; /* power-state in transition */ int power_count; /* current (global) power refcount */ struct delayed_work power_work; /* delayed task for powerdown */ diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 78e1827..d116908 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -98,6 +98,14 @@ struct hdmi_spec { */ struct hda_multi_out multiout; struct hda_pcm_stream pcm_playback; + +#ifdef CONFIG_PM + /* + * Non-generic Intel Haswell specific + */ + unsigned int ready_to_resume:1; + wait_queue_head_t resume_wq; +#endif }; @@ -977,6 +985,13 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) if (pin_idx < 0) return; +#ifdef CONFIG_PM + if (codec->resume_delayed) { + spec->ready_to_resume = 1; + wake_up(&spec->resume_wq); + } +#endif + hdmi_present_sense(&spec->pins[pin_idx], 1); snd_hda_jack_report_sync(codec); } @@ -1846,6 +1861,63 @@ static const struct hda_fixup hdmi_fixups[] = { }; +#ifdef CONFIG_PM +static void intel_haswell_wait_ready_to_resume(struct hda_codec *codec) +{ + struct hdmi_spec *spec = codec->spec; + int pin_idx; + + spec->ready_to_resume = 0; + + for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { + struct hdmi_spec_per_pin *per_pin; + hda_nid_t pin_nid; + struct hda_jack_tbl *jack; + + per_pin = &spec->pins[pin_idx]; + pin_nid = per_pin->pin_nid; + jack = snd_hda_jack_tbl_get(codec, pin_nid); + if (jack) + snd_hda_codec_write(codec, pin_nid, 0, + AC_VERB_SET_UNSOLICITED_ENABLE, + AC_USRSP_EN | jack->tag); + } + + wait_event_timeout(spec->resume_wq, + spec->ready_to_resume, msecs_to_jiffies(300)); + if (!spec->ready_to_resume) + snd_printd(KERN_WARNING "HDMI: Haswell not ready to resume\n"); +} + +static void intel_haswell_set_power_state(struct hda_codec *codec, hda_nid_t fg, + unsigned int power_state) +{ + if (codec->resume_delayed && power_state == AC_PWRST_D0) { + intel_haswell_wait_ready_to_resume(codec); + codec->resume_delayed = 0; + } + + snd_hda_codec_read(codec, fg, 0, + AC_VERB_SET_POWER_STATE, + power_state); + + snd_hda_codec_set_power_to_all(codec, fg, power_state); +} + +static inline void intel_haswell_allow_delay_resume(struct hda_codec *codec) +{ + struct hdmi_spec *spec = codec->spec; + + init_waitqueue_head(&spec->resume_wq); + codec->support_delay_resume = 1; + + codec->patch_ops.set_power_state = + intel_haswell_set_power_state; +} +#else +define intel_haswell_allow_delay_resume NULL +#endif + static int patch_generic_hdmi(struct hda_codec *codec) { struct hdmi_spec *spec; @@ -1868,6 +1940,10 @@ static int patch_generic_hdmi(struct hda_codec *codec) return -EINVAL; } codec->patch_ops = generic_hdmi_patch_ops; + + if (codec->vendor_id == 0x80862807) + intel_haswell_allow_delay_resume(codec); + generic_hdmi_init_per_pins(codec); init_channel_allocations(); -- 1.7.10.4

4 51

[alsa-devel] [PATCH] alsactl: sprintf to snprintf prevent buffer overflow
by Doug Goldstein 06 May '13

06 May '13

sprintf() is a bit dangerous unless you explicitly know your type size and want to keep it in sync always. Its safer to just use snprintf() and ensure your string doesn't overflow and is NULL terminated. Signed-off-by: Doug Goldstein <cardoe(a)cardoe.com> --- This was written to fix the buffer overflow crash in alsactl with the 1.0.27 release, which is documented in Gentoo bug #468160 [1]. The issue is actually already fixed upstream by 95788fea25c1a59985828d4b91af0772d077600b however the fix just changes the size and continues to use sprintf() which IMHO is still dangerous. The attached patch will fix the issue in a safer manner for good. [1] https://bugs.gentoo.org/show_bug.cgi?id=468160 --- alsactl/lock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/alsactl/lock.c b/alsactl/lock.c index fce208b..587a109 100644 --- a/alsactl/lock.c +++ b/alsactl/lock.c @@ -53,9 +53,9 @@ static int state_lock_(const char *file, int lock, int timeout) lck.l_len = 11; lck.l_pid = 0; if (lock) { - sprintf(lcktxt, "%10li\n", (long)getpid()); + snprintf(lcktxt, sizeof(lcktxt), "%10li\n", (long)getpid()); } else { - sprintf(lcktxt, "%10s\n", ""); + snprintf(lcktxt, sizeof(lcktxt), "%10s\n", ""); } while (fd < 0 && timeout-- > 0) { fd = open(nfile, O_RDWR); -- 1.8.2.1

2 1

[alsa-devel] [PATCH 1/2] ALSA: mips/hal2: Remove redundant platform_set_drvdata()
by Sachin Kamat 06 May '13

06 May '13

Commit 0998d06310 (device-core: Ensure drvdata = NULL when no driver is bound) removes the need to set driver data field to NULL. Signed-off-by: Sachin Kamat <sachin.kamat(a)linaro.org> Cc: Thomas Bogendoerfer <tsbogend(a)alpha.fanken.de> --- sound/mips/hal2.c | 1 - 1 file changed, 1 deletion(-) diff --git a/sound/mips/hal2.c b/sound/mips/hal2.c index 7420c59..2b7f6e8 100644 --- a/sound/mips/hal2.c +++ b/sound/mips/hal2.c @@ -922,7 +922,6 @@ static int hal2_remove(struct platform_device *pdev) struct snd_card *card = platform_get_drvdata(pdev); snd_card_free(card); - platform_set_drvdata(pdev, NULL); return 0; } -- 1.7.9.5

2 3