December 2016 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] [PATCH] ASoC: sun4i-i2s: Add quirks for newer SoCs
by codekipper＠gmail.com 20 Dec '16

20 Dec '16

From: Marcus Cooper <codekipper(a)gmail.com> Newer SoCs have additional functionality so a quirks structure has been added to handle them. So far we've seen the use of a reset controller, a different address for the TXFIFO and varying register changes. This patch prepares the driver for these changes and adds the reset specifier. Signed-off-by: Marcus Cooper <codekipper(a)gmail.com> --- .../devicetree/bindings/sound/sun4i-i2s.txt | 2 + sound/soc/sunxi/sun4i-i2s.c | 47 ++++++++++++++++++++-- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt index 7a2c0945fd22..494a881ccd21 100644 --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt @@ -18,6 +18,8 @@ Required properties: - "apb" : clock for the I2S bus interface - "mod" : module clock for the I2S controller - #sound-dai-cells : Must be equal to 0 +- resets: reset specifier for the ahb reset (A31 and newer only) + Example: diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index f24d19526603..80fe4f1d6e3b 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -14,9 +14,11 @@ #include <linux/clk.h> #include <linux/dmaengine.h> #include <linux/module.h> +#include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/regmap.h> +#include <linux/reset.h> #include <sound/dmaengine_pcm.h> #include <sound/pcm_params.h> @@ -92,6 +94,7 @@ struct sun4i_i2s { struct clk *bus_clk; struct clk *mod_clk; struct regmap *regmap; + struct reset_control *rst; unsigned int mclk_freq; @@ -104,6 +107,13 @@ struct sun4i_i2s_clk_div { u8 val; }; +struct sun4i_i2s_quirks { + unsigned int reg_dac_txdata; /* TX FIFO offset for DMA config */ + bool has_reset; + const struct regmap_config *sun4i_i2s_regmap; + const struct snd_soc_dai_ops *ops; +}; + static const struct sun4i_i2s_clk_div sun4i_i2s_bclk_div[] = { { .div = 2, .val = 0 }, { .div = 4, .val = 1 }, @@ -541,7 +551,6 @@ static struct snd_soc_dai_driver sun4i_i2s_dai = { .rates = SNDRV_PCM_RATE_8000_192000, .formats = SNDRV_PCM_FMTBIT_S16_LE, }, - .ops = &sun4i_i2s_dai_ops, .symmetric_rates = 1, }; @@ -655,6 +664,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev) { struct sun4i_i2s *i2s; struct resource *res; + const struct sun4i_i2s_quirks *quirks; void __iomem *regs; int irq, ret; @@ -680,8 +690,14 @@ static int sun4i_i2s_probe(struct platform_device *pdev) return PTR_ERR(i2s->bus_clk); } + quirks = of_device_get_match_data(&pdev->dev); + if (quirks == NULL) { + dev_err(&pdev->dev, "Failed to determine the quirks to use\n"); + return -ENODEV; + } + i2s->regmap = devm_regmap_init_mmio(&pdev->dev, regs, - &sun4i_i2s_regmap_config); + quirks->sun4i_i2s_regmap); if (IS_ERR(i2s->regmap)) { dev_err(&pdev->dev, "Regmap initialisation failed\n"); return PTR_ERR(i2s->regmap); @@ -692,13 +708,25 @@ static int sun4i_i2s_probe(struct platform_device *pdev) dev_err(&pdev->dev, "Can't get our mod clock\n"); return PTR_ERR(i2s->mod_clk); } + - i2s->playback_dma_data.addr = res->start + SUN4I_I2S_FIFO_TX_REG; + i2s->playback_dma_data.addr = res->start + quirks->reg_dac_txdata; i2s->playback_dma_data.maxburst = 4; i2s->capture_dma_data.addr = res->start + SUN4I_I2S_FIFO_RX_REG; i2s->capture_dma_data.maxburst = 4; + if (quirks->has_reset) { + i2s->rst = devm_reset_control_get_optional(&pdev->dev, NULL); + if (IS_ERR(i2s->rst) && PTR_ERR(i2s->rst) == -EPROBE_DEFER) { + ret = -EPROBE_DEFER; + dev_err(&pdev->dev, "Failed to get reset: %d\n", ret); + goto err_pm_disable; + } + if (!IS_ERR(i2s->rst)) + reset_control_deassert(i2s->rst); + } + pm_runtime_enable(&pdev->dev); if (!pm_runtime_enabled(&pdev->dev)) { ret = sun4i_i2s_runtime_resume(&pdev->dev); @@ -706,6 +734,8 @@ static int sun4i_i2s_probe(struct platform_device *pdev) goto err_pm_disable; } + /* Register ops with dai */ + sun4i_i2s_dai.ops = quirks->ops; ret = devm_snd_soc_register_component(&pdev->dev, &sun4i_i2s_component, &sun4i_i2s_dai, 1); @@ -742,8 +772,17 @@ static int sun4i_i2s_remove(struct platform_device *pdev) return 0; } +static const struct sun4i_i2s_quirks sun4i_a10_i2s_quirks = { + .reg_dac_txdata = SUN4I_I2S_FIFO_TX_REG, + .sun4i_i2s_regmap = &sun4i_i2s_regmap_config, + .ops = &sun4i_i2s_dai_ops, +}; + static const struct of_device_id sun4i_i2s_match[] = { - { .compatible = "allwinner,sun4i-a10-i2s", }, + { + .compatible = "allwinner,sun4i-a10-i2s", + .data = &sun4i_a10_i2s_quirks, + }, {} }; MODULE_DEVICE_TABLE(of, sun4i_i2s_match); -- 2.11.0

3 2

[alsa-devel] [PATCH v3 0/4] Generic HDMI codec: Add channel mapping control
by Arnaud Pouliquen 20 Dec '16

20 Dec '16

Aim of this patch is to add 'Playback Channel Map' control to export audio capabilities in term of HDMI sink speakers allocation. V3: - "ASoC: hdmi-codec: add channel mapping control": => Minor fixes: - select stereo speaker config by default if HDMI cable unplugged - fix compilation warning. - "DRM: add help to get ELD speaker allocation" => No delta vs V2. - "ALSA pcm: allow non constant snd_pcm_chmap_elem" => No delta vs V2. - "ASoC: core: add optional pcm_new callback for DAI driver" => No delta vs V2. V2: In this version I use chmap helper functions from pcm_lib.c. It is quite tricky to use it from ASoC due to the relation chip of the controls with the pcm runtime. After several tries, my conclusion is that it must be handled in ASoC DAI driver. Main reason is that the DAI driver can not provide snd_pcm_chmap struct through kcontrol structure. But this induces that soc-core provides pcm runtime structure to DAI driver during probe. Base on this conclusion. I reworked my patches by adding 2 new patches in patch-set 1) ALSA pcm: allow non constant snd_pcm_chmap_elem This patch allows to handle non constant channel mapping. As ELD information can change during runtime it is mandatory to properly handle the feature. In term of compatibility with legacy it should be straightforward, as update just consists in suppressing the 'const' constraint. 2) ASoC: core: add optional pcm_new callback for DAI driver This is the only way i found to be able to use snd_pcm_add_chmap_ctls and associated controls helper functions. From my point of view, this is the more simple way to add relationship between DAI and associated pcm devices. Notice that this patch, if accepted, makes the following one obsolete, as it also answer to the associated topic: [PATCH v5 0/2] ALSA controls management using index/device/sub-devices fields (http://www.spinics.net/lists/alsa-devel/msg57639.html). If you estimate that this it not reasonable i will come back to my first version. V1: This patch follows discussion initiate here: [RFC] ASOC: HDMI audio info frame speaker allocation http://www.spinics.net/lists/alsa-devel/msg57363.html The code is fully inspired from HDA driver. On hw_param, HDMI sink speaker capabilities are exported via TLV ops and a CEA allocation is choson, based on ELD information and the number of channels requested by user. Mains differences with HDA implementation are: - Control is read only - Channel swap is not supported. Consequence is that unused channel in the mid of CEA audio infoframe channel mapping are considered as active. example for channel allocation 0x02: FL, FR, 0, FC) This configuration is only available for a 4 channels stream. - Channel allocation table has been reordered and HDMI 2.0 is not supported. Arnaud Pouliquen (4): DRM: add help to get ELD speaker allocation ALSA pcm: allow non constant snd_pcm_chmap_elem ASoC: core: add optional pcm_new callback for DAI driver ASoC: hdmi-codec: add channel mapping control include/drm/drm_edid.h | 13 ++ include/sound/pcm.h | 4 +- include/sound/soc-dai.h | 3 + sound/core/pcm_lib.c | 6 +- sound/soc/codecs/hdmi-codec.c | 313 +++++++++++++++++++++++++++++++++++++++++- sound/soc/soc-core.c | 28 ++++ 6 files changed, 361 insertions(+), 6 deletions(-) -- 1.9.1

4 8

[alsa-devel] [PATCH v2] ASoC: wm_adsp: Don't overrun firmware file buffer when reading region data
by Richard Fitzgerald 20 Dec '16

20 Dec '16

Protect against corrupt firmware files by ensuring that the length we get for the data in a region actually lies within the available firmware file data buffer. Signed-off-by: Richard Fitzgerald <rf(a)opensource.wolfsonmicro.com> --- sound/soc/codecs/wm_adsp.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index 593b7d1..d72ccef 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -1551,7 +1551,7 @@ static int wm_adsp_load(struct wm_adsp *dsp) const struct wmfw_region *region; const struct wm_adsp_region *mem; const char *region_name; - char *file, *text; + char *file, *text = NULL; struct wm_adsp_buf *buf; unsigned int reg; int regions = 0; @@ -1700,10 +1700,21 @@ static int wm_adsp_load(struct wm_adsp *dsp) regions, le32_to_cpu(region->len), offset, region_name); + if ((pos + le32_to_cpu(region->len) + sizeof(*region)) > + firmware->size) { + adsp_err(dsp, + "%s.%d: %s region len %d bytes exceeds file length %zu\n", + file, regions, region_name, + le32_to_cpu(region->len), firmware->size); + ret = -EINVAL; + goto out_fw; + } + if (text) { memcpy(text, region->data, le32_to_cpu(region->len)); adsp_info(dsp, "%s: %s\n", file, text); kfree(text); + text = NULL; } if (reg) { @@ -1748,6 +1759,7 @@ static int wm_adsp_load(struct wm_adsp *dsp) regmap_async_complete(regmap); wm_adsp_buf_free(&buf_list); release_firmware(firmware); + kfree(text); out: kfree(file); @@ -2233,6 +2245,17 @@ static int wm_adsp_load_coeff(struct wm_adsp *dsp) } if (reg) { + if ((pos + le32_to_cpu(blk->len) + sizeof(*blk)) > + firmware->size) { + adsp_err(dsp, + "%s.%d: %s region len %d bytes exceeds file length %zu\n", + file, blocks, region_name, + le32_to_cpu(blk->len), + firmware->size); + ret = -EINVAL; + goto out_fw; + } + buf = wm_adsp_buf_alloc(blk->data, le32_to_cpu(blk->len), &buf_list); -- 1.9.1

2 1

Re: [alsa-devel] [PATCH] ALSA: snd-usb: fix IRQ triggered NULL pointer dereference
by Takashi Sakamoto 20 Dec '16

20 Dec '16

--- Hi, > Commit 16200948d835 ("ALSA: usb-audio: Fix race at stopping the stream") > fixes a race-codition but it also introduces another really nasty data race > regression which makes my usb sound card [1] completely useless, throwing > the kernel into a panic if anything from userspace tries to start playback. > > The problem is this: ep->data_subs is now set to NULL every time inside > wait_clear_urbs(). ep->data_subs is initalized only in one place in > start_endpoints(), then it is immediately wiped by the pre-existing call to > wait_clear_urbs() inside snd_usb_endpoint_start(). > > To ilustrate, this is what happens in the non-irq part of the code: > > Step 1 (inside start_endpoints): ep->data_subs = subs; > Step 2 (inside start_endpoints): snd_usb_endpoint_start(ep, can_sleep); > Step 3 (inside snd_usb_endpoint_start): wait_clear_urbs(ep); > Step 4 (inside wait_clear_urbs): ep->data_subs = NULL; > > Here's a simplified call stack for the IRQ part (full one at the end): > > (NULL dereference, param "subs" is passed NULL value of ep->data_subs) > retire_playback_urb > retire_outbound_urb > snd_complete_urb > (...) > xhci_irq > > Looking at the git log there seems to be quite a lot of history in this > part of the codebase, dating back to 2012 or earlier. My fix is based on > 015618b90 ("ALSA: snd-usb: Fix URB cancellation at stream start") and > e9ba389c5 ("ALSA: usb-audio: Fix scheduling-while-atomic bug in PCM capture > stream") with a few modifications of my own. My idea is to do the > ep->data_subs wiping before endpoint initialization in a manner similar > to these older commits by using stop_endpoints() in snd_usb_pcm_prepare() > and at the same time keep the ep->data_subs = NULL in wait_clear_urbs() to > not trigger the recently fixed stream stopping race again. > > Full call stack: > > [ 131.093240] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010 > [ 131.094313] IP: retire_playback_urb+0x1b/0x160 [snd_usb_audio] > [ 131.095046] PGD 0 > [ 131.095047] > [ 131.096509] Oops: 0000 [#1] PREEMPT SMP > [ 131.097255] Modules linked in: fuse snd_usb_audio snd_usbmidi_lib snd_rawmidi snd_seq_device ctr ccm arc4 ath9k intel_rapl ath9k_common x86_pkg_temp_thermal ath9k_hw intel_powerclamp coretemp joydev mousedev ath kvm_intel mac80211 kvm > input_leds hid_generic psmouse irqbypass usbhid hid crct10dif_pclmul crc32_pclmul serio_raw crc32c_intel atkbd ghash_clmulni_intel snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel pcbc aesni_intel libps2 cfg8 > 0211 aes_x86_64 crypto_simd dell_laptop glue_helper r8169 dell_smbios snd_hda_codec sr_mod cryptd led_class mii rfkill snd_hwdep cdrom snd_hda_core ac dcdbas i8042 xhci_pci serio xhci_hcd snd_pcm tpm_tis battery tpm_tis_core lpc_ich tpm > snd_timer evdev shpchp i2c_i801 sch_fq_codel > [ 131.105551] CPU: 2 PID: 165 Comm: irq/29-xhci_hcd Not tainted 4.9.0-gd824cdc58ba0 #10 > [ 131.107516] Hardware name: Dell Inc. Inspiron 3521/018DYG, BIOS A14 07/31/2015 > [ 131.109592] task: ffff880154a70000 task.stack: ffffc90000f48000 > [ 131.111746] RIP: 0010:retire_playback_urb+0x1b/0x160 [snd_usb_audio] > [ 131.113899] RSP: 0018:ffffc90000f4bc10 EFLAGS: 00010082 > [ 131.116080] RAX: ffffffffa04cabe0 RBX: 0000000000000000 RCX: 0000000000000000 > [ 131.118284] RDX: 0000000000000000 RSI: ffff8801435a4c00 RDI: 0000000000000000 > [ 131.120505] RBP: ffffc90000f4bc40 R08: 0000000000000001 R09: 0000000000000001 > [ 131.122807] R10: 0000000000000001 R11: ffffffff82f60d6d R12: ffff8801432a0238 > [ 131.125265] R13: ffff8801435a4c00 R14: 0000000000000000 R15: ffff8801535c83b8 > [ 131.127723] FS: 0000000000000000(0000) GS:ffff88015a000000(0000) knlGS:0000000000000000 > [ 131.130217] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 131.132568] CR2: 0000000000000010 CR3: 0000000001e0b000 CR4: 00000000001406e0 > [ 131.135007] Call Trace: > [ 131.137329] snd_complete_urb+0x190/0x2b0 [snd_usb_audio] > [ 131.139526] __usb_hcd_giveback_urb+0x8e/0x160 > [ 131.141948] usb_hcd_giveback_urb+0x43/0x110 > [ 131.144104] xhci_giveback_urb_in_irq.isra.22+0x7d/0xb0 [xhci_hcd] > [ 131.146521] finish_td.constprop.38+0x1de/0x2f0 [xhci_hcd] > [ 131.148736] xhci_irq+0x13a2/0x1ca0 [xhci_hcd] > [ 131.150972] ? trace_hardirqs_on+0xd/0x10 > [ 131.153209] ? _raw_spin_unlock_irq+0x2c/0x50 > [ 131.155390] ? irq_thread+0xb5/0x1d0 > [ 131.157771] xhci_msi_irq+0x11/0x20 [xhci_hcd] > [ 131.159938] irq_forced_thread_fn+0x2f/0x70 > [ 131.162072] ? irq_thread+0xb5/0x1d0 > [ 131.164141] irq_thread+0x149/0x1d0 > [ 131.166132] ? irq_finalize_oneshot.part.2+0xe0/0xe0 > [ 131.168142] ? wake_threads_waitq+0x30/0x30 > [ 131.170149] kthread+0x10f/0x150 > [ 131.172144] ? irq_thread_dtor+0xc0/0xc0 > [ 131.174139] ? kthread_create_on_node+0x40/0x40 > [ 131.176116] ret_from_fork+0x2a/0x40 > [ 131.178074] Code: 8b 77 64 4c 89 e7 e8 e5 fe ff ff eb c3 0f 1f 00 0f 1f 44 00 00 55 31 d2 48 89 e5 41 57 41 56 41 55 41 54 53 48 89 fb 48 83 ec 08 <48> 8b 47 10 48 8b 4f 70 48 c7 c7 88 7b 4d a0 4c 8b a0 78 01 00 > [ 131.182382] RIP: retire_playback_urb+0x1b/0x160 [snd_usb_audio] RSP: ffffc90000f4bc10 > [ 131.184562] CR2: 0000000000000010 > > [1] 041e:3232 Creative Technology SoundBlaster X-FI HD > [2] http://mailman.alsa-project.org/pipermail/alsa-devel/2016-December/115425.h… I can regenerate this bug with my EMU 0404 USB. My understanding of this bug is quite similar to your perspective. This bug is quite critical. In my understanding, we encounter this bug in all of cases in which snd-usb-audio is applied for PCM playback substream. Below workaround can also suppress this bug. diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index a2cdf33..9b34f76 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -537,11 +537,14 @@ static int wait_clear_urbs(struct snd_usb_endpoint *ep) alive, ep->ep_num); clear_bit(EP_FLAG_STOPPING, &ep->flags); - ep->data_subs = NULL; ep->sync_slave = NULL; ep->retire_data_urb = NULL; ep->prepare_data_urb = NULL; + msleep(200); + + ep->data_subs = NULL; + return 0; } If initialization of 'struct snd_usb_endpoint.data_subs' can be done after all of queued URBs are processed and corresponding complete interrupts are cought, we can solve this critical bug. > Signed-off-by: Ioan-Adrian Ratiu <adi(a)adirat.com> > --- > sound/usb/endpoint.c | 11 ++--------- > sound/usb/endpoint.h | 2 +- > sound/usb/pcm.c | 13 ++++++------- > 3 files changed, 9 insertions(+), 17 deletions(-) > > diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c > index a2cdf3370afe..4465f324c2c2 100644 > --- a/sound/usb/endpoint.c > +++ b/sound/usb/endpoint.c > @@ -920,9 +920,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep, > /** > * snd_usb_endpoint_start: start an snd_usb_endpoint > * > - * @ep: the endpoint to start > - * @can_sleep: flag indicating whether the operation is executed in > - * non-atomic context > + * @ep: the endpoint to start > * > * A call to this function will increment the use count of the endpoint. > * In case it is not already running, the URBs for this endpoint will be > @@ -932,7 +930,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep, > * > * Returns an error if the URB submission failed, 0 in all other cases. > */ > -int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep) > +int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) > { > int err; > unsigned int i; > @@ -944,11 +942,6 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep) > if (++ep->use_count != 1) > return 0; > > - /* just to be sure */ > - deactivate_urbs(ep, false); > - if (can_sleep) > - wait_clear_urbs(ep); > - > ep->active_mask = 0; > ep->unlink_mask = 0; > ep->phase = 0; > diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h > index 6428392d8f62..584f295d7c77 100644 > --- a/sound/usb/endpoint.h > +++ b/sound/usb/endpoint.h > @@ -18,7 +18,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep, > struct audioformat *fmt, > struct snd_usb_endpoint *sync_ep); > > -int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep); > +int snd_usb_endpoint_start(struct snd_usb_endpoint *ep); > void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep); > void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep); > int snd_usb_endpoint_activate(struct snd_usb_endpoint *ep); > diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c > index 34c6d4f2c0b6..db26f767f851 100644 > --- a/sound/usb/pcm.c > +++ b/sound/usb/pcm.c > @@ -218,7 +218,7 @@ int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface, > } > } > > -static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep) > +static int start_endpoints(struct snd_usb_substream *subs) > { > int err; > > @@ -231,7 +231,7 @@ static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep) > dev_dbg(&subs->dev->dev, "Starting data EP @%p\n", ep); > > ep->data_subs = subs; > - err = snd_usb_endpoint_start(ep, can_sleep); > + err = snd_usb_endpoint_start(ep); > if (err < 0) { > clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags); > return err; > @@ -260,7 +260,7 @@ static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep) > dev_dbg(&subs->dev->dev, "Starting sync EP @%p\n", ep); > > ep->sync_slave = subs->data_endpoint; > - err = snd_usb_endpoint_start(ep, can_sleep); > + err = snd_usb_endpoint_start(ep); > if (err < 0) { > clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags); > return err; > @@ -809,8 +809,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) > goto unlock; > } > > - snd_usb_endpoint_sync_pending_stop(subs->sync_endpoint); > - snd_usb_endpoint_sync_pending_stop(subs->data_endpoint); > + stop_endpoints(subs, true); > > ret = set_format(subs, subs->cur_audiofmt); > if (ret < 0) > @@ -850,7 +849,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) > /* for playback, submit the URBs now; otherwise, the first hwptr_done > * updates for all URBs would happen at the same time when starting */ > if (subs->direction == SNDRV_PCM_STREAM_PLAYBACK) > - ret = start_endpoints(subs, true); > + return start_endpoints(subs); > > unlock: > snd_usb_unlock_shutdown(subs->stream->chip); > @@ -1666,7 +1665,7 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream > > switch (cmd) { > case SNDRV_PCM_TRIGGER_START: > - err = start_endpoints(subs, false); > + err = start_endpoints(subs); > if (err < 0) > return err; This patch works better, but not the best. It's a bit intrusive. What we need to synchronize process contexts to interrupt context till all of queued URBs are handled. This seems to be a main purpose of wait_clear_bits, while current implementation of snd_complete_urb()/wait_clear_bits() is not enough. Thanks for your reporting. Takashi Sakamoto -- 2.9.3

1 0

[alsa-devel] [PATCH v3 2/2] ASoC: cs43130: Add devicetree bindings for CS43130
by Li Xu 19 Dec '16

19 Dec '16

Add devicetree bindings documentation file for Cirrus Logic CS43130 codec. Signed-off-by: Li Xu <li.xu(a)cirrus.com> --- .../devicetree/bindings/sound/cs43130.txt | 41 ++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/cs43130.txt diff --git a/Documentation/devicetree/bindings/sound/cs43130.txt b/Documentation/devicetree/bindings/sound/cs43130.txt new file mode 100644 index 0000000..1af6b78 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/cs43130.txt @@ -0,0 +1,41 @@ +CS43130 DAC + +Required properties: + + - compatible : "cirrus,cs43130" + + - reg : the I2C address of the device for I2C + + - VA-supply, VP-supply, VL-supply, VCP-supply, VD-supply: + power supplies for the device, as covered in + Documentation/devicetree/bindings/regulator/regulator.txt. + + +Optional properties: + + - reset-gpios : Active low GPIO used to reset the device + + - cirrus,xtal-ibias: + When external MCLK is generated by external crystal + oscillator, CS43130 can be used to provide bias current + for external crystal. Amount of bias current sent is + set as: + 1 = 7.5uA + 2 = 12.5uA + 3 = 15uA + +Example: + +cs43130: audio-codec@30 { + compatible = "cirrus,cs43130"; + reg = <0x30>; + reset-gpios = <&axi_gpio 54 1>; + VA-supply = <&dummy_vreg>; + VP-supply = <&dummy_vreg>; + VL-supply = <&dummy_vreg>; + VCP-supply = <&dummy_vreg>; + VD-supply = <&dummy_vreg>; + cirrus,xtal-ibias = <2>; + interrupt-parent = <&gpio0>; + interrupts = <55 8>; +}; -- 1.9.1

2 1

[alsa-devel] [PATCH v2 2/2] ASoC: cs35l35: Add device tree documentation for CS35L35
by Li Xu 19 Dec '16

19 Dec '16

Add device tree documentation for Cirrus Logic CS35L35 speaker amplifier Signed-off-by: Li Xu <li.xu(a)cirrus.com> --- .../devicetree/bindings/sound/cs35l35.txt | 172 +++++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/cs35l35.txt diff --git a/Documentation/devicetree/bindings/sound/cs35l35.txt b/Documentation/devicetree/bindings/sound/cs35l35.txt new file mode 100644 index 0000000..8b13f67 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/cs35l35.txt @@ -0,0 +1,172 @@ +CS35L35 Speaker Amplifier + +Required properties: + + - compatible : "cirrus,cs35l35" + + - reg : the I2C address of the device for I2C + + - VA-supply, VP-supply : power supplies for the device, + as covered in + Documentation/devicetree/bindings/regulator/regulator.txt. + + - interrupt-parent : Specifies the phandle of the interrupt controller to + which the IRQs from CS35L35 are delivered to. + - interrupts : IRQ line info CS35L35. + (See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt + for further information relating to interrupt properties) + +Optional properties: + - cirrus,reset-gpios : Active low GPIO used to reset the amplifier + + - cirrus,stereo-config : Boolean to determine if there are 2 AMPs for a + Stereo configuration + + - cirrus,audio-channel : Set Location of Audio Signal on Serial Port + 0 = Data Packet received on Left I2S Channel + 1 = Data Packet received on Right I2S Channel + + - cirrus,advisory-channel : Set Location of Advisory Signal on Serial Port + 0 = Data Packet received on Left I2S Channel + 1 = Data Packet received on Right I2S Channel + + - cirrus,shared-boost : Boolean to enable ClassH tracking of Advisory Signal + if 2 Devices share Boost BST_CTL + + - cirrus,sp-drv-strength : Value for setting the Serial Port drive strength + Table 3-10 of the datasheet lists drive-strength specifications + 0 = 1x (Default) + 1 = .5x + + - cirrus,bst-pdn-fet-on : Boolean to determine if the Boost PDN control + powers down with a rectification FET On or Off. If VSPK is supplied + externally then FET is off. + + - cirrus,boost-ctl-millivolt : Boost Converter control word. Step Size of 100mV + 0x00 = (Default) VP + 0x01 = 2600mV + 0x41 = 9000mV + + - cirrus,boost-ipk-milliamp : Boost-converter peak current limit. + Configures the peak current by monitoring the current through the boost FET. + Step size: 112mA + 0x00 = 1680mA + 0x19 = 4480mA + + - cirrus,amp-gain-zc : Boolean to determine if to use Amplifier gain-change + zero-cross + +Optional H/G Algorithm sub-node: + + The cs35l35 node can have a single "cirrus,classh-internal-algo" sub-node + that will disable automatic control of the internal H/G Algorithm. + + It is strongly recommended that the Datasheet be referenced when adjusting + or using these Class H Algorithm controls over the internal Algorithm. + Serious damage can occur to the Device and surrounding components. + + - cirrus,classh-internal-algo : Sub-node for the Internal Class H Algorithm + See Section 4.3 Internal Class H Algorithm in the Datasheet. + If not used, the device manages the ClassH Algorithm internally. + +Optional properties for the "cirrus,classh-internal-algo" Sub-node + + Section 7.29 Class H Control + - cirrus,classh-bst-overide : Boolean + - cirrus,classh-bst-max-limit + - cirrus,classh-mem-depth + + Section 7.30 Class H Headroom Control + - cirrus,classh-headroom + + Section 7.31 Class H Release Rate + - cirrus,classh-release-rate + + Section 7.32 Class H Weak FET Drive Control + - cirrus,classh-wk-fet-disable + - cirrus,classh-wk-fet-delay + - cirrus,classh-wk-fet-thld + + Section 7.34 Class H VP Control + - cirrus,classh-vpch-auto + - cirrus,classh-vpch-rate + - cirrus,classh-vpch-man + +Optional Monitor Signal Format sub-node: + + The cs35l35 node can have a single "cirrus,monitor-signal-format" sub-node + for adjusting the Depth, Location and Frame of the Monitoring Signals + for Algorithms. + + See Sections 4.8.2 through 4.8.4 Serial-Port Control in the Datasheet + + -cirrus,monitor-signal-format : Sub-node for the Monitor Signaling Formating + on the I2S Port. Each of the 3 8 bit values in the array contain the settings + for depth, location, and frame. + + If not used, the defaults for the 6 monitor signals is used. + + Sections 7.44 - 7.53 lists values for the depth, location, and frame + for each monitoring signal. + + - cirrus,imon : 3 8 bit values to set the depth, location, and frame + of the IMON monitor signal. + + - cirrus,vmon : 3 8 bit values to set the depth, location, and frame + of the VMON monitor signal. + + - cirrus,vpmon : 3 8 bit values to set the depth, location, and frame + of the VPMON monitor signal. + + - cirrus,vbstmon : 3 8 bit values to set the depth, location, and frame + of the VBSTMON monitor signal + + - cirrus,vpbrstat : 3 8 bit values to set the depth, location, and frame + of the VPBRSTAT monitor signal + + - cirrus,zerofill : 3 8 bit values to set the depth, location, and frame\ + of the ZEROFILL packet in the monitor signal + +Example: + +cs35l35: audio-codec@20 { + compatible = "cirrus,cs35l35"; + reg = <0x20>; + VA-supply = <&dummy_vreg>; + VP-supply = <&dummy_vreg>; + reset-gpios = <&axi_gpio 54 1>; + interrupt-parent = <&gpio8>; + interrupts = <3 IRQ_TYPE_LEVEL_LOW>; + cirrus,boost-ctl = <0x41>; + + cirrus,stereo-config { + cirrus,audio-channel = <0x00>; + cirrus,advisory-channel = <0x01>; + cirrus,shared-boost; + }; + + cirrus,classh-internal-algo { + cirrus,classh-bst-overide; + cirrus,classh-bst-max-limit = <0x01>; + cirrus,classh-mem-depth = <0x01>; + cirrus,classh-release-rate = <0x08>; + cirrus,classh-headroom-millivolt = <0x0B>; + cirrus,classh-wk-fet-disable = <0x01>; + cirrus,classh-wk-fet-delay = <0x04>; + cirrus,classh-wk-fet-thld = <0x01>; + cirrus,classh-vpch-auto = <0x01>; + cirrus,classh-vpch-rate = <0x02>; + cirrus,classh-vpch-man = <0x05>; + }; + + /* Depth, Location, Frame */ + cirrus,monitor-signal-format { + cirrus,imon = /bits/ 8 <0x03 0x00 0x01>; + cirrus,vmon = /bits/ 8 <0x03 0x00 0x00>; + cirrus,vpmon = /bits/ 8 <0x03 0x04 0x00>; + cirrus,vbstmon = /bits/ 8 <0x03 0x04 0x01>; + cirrus,vpbrstat = /bits/ 8 <0x00 0x04 0x00>; + cirrus,zerofill = /bits/ 8 <0x00 0x00 0x00>; + }; + +}; -- 1.9.1

2 1

[alsa-devel] [PATCH] ASoC: wm_adsp: Don't overrun firmware file buffer when reading region data
by Richard Fitzgerald 19 Dec '16

19 Dec '16

Protect against corrupt firmware files by ensuring that the length we get for the data in a region actually lies within the available firmware file data buffer. Signed-off-by: Richard Fitzgerald <rf(a)opensource.wolfsonmicro.com> --- sound/soc/codecs/wm_adsp.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index 593b7d1..91ccf62 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -1551,7 +1551,7 @@ static int wm_adsp_load(struct wm_adsp *dsp) const struct wmfw_region *region; const struct wm_adsp_region *mem; const char *region_name; - char *file, *text; + char *file, *text = NULL; struct wm_adsp_buf *buf; unsigned int reg; int regions = 0; @@ -1700,10 +1700,21 @@ static int wm_adsp_load(struct wm_adsp *dsp) regions, le32_to_cpu(region->len), offset, region_name); + if ((pos + le32_to_cpu(region->len) + sizeof(*region)) > + firmware->size) { + adsp_err(dsp, + "%s.%d: %s region len %d bytes exceeds file length %d\n", + file, regions, region_name, + le32_to_cpu(region->len), firmware->size); + ret = -EINVAL; + goto out_fw; + } + if (text) { memcpy(text, region->data, le32_to_cpu(region->len)); adsp_info(dsp, "%s: %s\n", file, text); kfree(text); + text = NULL; } if (reg) { @@ -1748,6 +1759,7 @@ static int wm_adsp_load(struct wm_adsp *dsp) regmap_async_complete(regmap); wm_adsp_buf_free(&buf_list); release_firmware(firmware); + kfree(text); out: kfree(file); @@ -2233,6 +2245,17 @@ static int wm_adsp_load_coeff(struct wm_adsp *dsp) } if (reg) { + if ((pos + le32_to_cpu(blk->len) + sizeof(*blk)) > + firmware->size) { + adsp_err(dsp, + "%s.%d: %s region len %d bytes exceeds file length %d\n", + file, blocks, region_name, + le32_to_cpu(blk->len), + firmware->size); + ret = -EINVAL; + goto out_fw; + } + buf = wm_adsp_buf_alloc(blk->data, le32_to_cpu(blk->len), &buf_list); -- 1.9.1

1 0

[alsa-devel] [PATCH v3] ASoC: Intel: bytcr_rt5640: fallback mechanism if MCLK is not enabled
by Pierre-Louis Bossart 19 Dec '16

19 Dec '16

Commit df1a2776a795 ("ASoC: Intel: bytcr_rt5640: add MCLK support") was merged but the corresponding clock framework patches have not, after being bumped from audio to clock to x86 domains. The missing clock-related patches result in a regression starting with 4.9 with the audio card not being created. Rather than reverting this commit and all following updates already queued up for 4.10, handle run-time dependency on MCLK and fall back to the previous bit-clock mode. This provides the same functionality as in 4.8 for Baytrail devices. On Baytrail-CR most devices remain silent with this fallback but additional patches are needed anyway. As suggested by Mark Brown, the fallback is only allowed with -ENOENT, all other run-time errors, including -EPROBE_DEFER, will stop the probe with no sound card registered. This patch should be applied to -stable as well as ASoC 4.10 fixes Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> --- Notes: v3: fixed printf format warning reported by 0-day sound/soc/intel/boards/bytcr_rt5640.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index 507a86a..e33e477 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -825,10 +825,20 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) if ((byt_rt5640_quirk & BYT_RT5640_MCLK_EN) && (is_valleyview())) { priv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3"); if (IS_ERR(priv->mclk)) { + ret_val = PTR_ERR(priv->mclk); + dev_err(&pdev->dev, - "Failed to get MCLK from pmc_plt_clk_3: %ld\n", - PTR_ERR(priv->mclk)); - return PTR_ERR(priv->mclk); + "Failed to get MCLK from pmc_plt_clk_3: %d\n", + ret_val); + + /* + * Fall back to bit clock usage for -ENOENT (clock not + * available likely due to missing dependencies), bail + * for all other errors, including -EPROBE_DEFER + */ + if (ret_val != -ENOENT) + return ret_val; + byt_rt5640_quirk &= ~BYT_RT5640_MCLK_EN; } } -- 2.7.4

2 1

Re: [alsa-devel] [PATCH 4/4] hda: thinkpad_helper: Add support for hid-lenovo LED control
by Dennis Wassenberg 19 Dec '16

19 Dec '16

Hi Jiri, On 19.12.2016 11:53, Jiri Kosina wrote: > On Mon, 19 Dec 2016, Dennis Wassenberg wrote: > >> "led_set_func_hid_lenovo" is set to "hid_lenovo_led_set" >> >> +#if IS_ENABLED(CONFIG_HID_LENOVO) >> +static int hda_fixup_thinkpad_hid_prepare(struct hda_codec *codec) >> +{ >> + struct hda_gen_spec *spec = codec->spec; >> + int ret = 0; >> + >> + if (!is_thinkpad(codec)) >> + return -ENODEV; >> + if (!led_set_func_hid_lenovo) >> + led_set_func_hid_lenovo = symbol_request(hid_lenovo_led_set); >> >> "hid_lenovo_led_set" is introduced in "[PATCH v2 2/2] hda: >> thinkpad_helper: Add support for hid-lenovo LED control". This means >> that "[PATCH 4/4] hda: thinkpad_helper: Add support for hid-lenovo LED >> control" can only work with applied "[PATCH 1/4] hid-lenovo: Add support >> for X1 Tablet cover special keys" and "[PATCH v2 2/2] hda: >> thinkpad_helper: Add support for hid-lenovo LED control". > > Ah, okay, I wasn't aware of the other patchset. These two probably should > be merged together. Is the other 2-patch set merged in any tree already? > Where has it been submitted to? > No 1/4, 2/4 and 3/4 are currently not merged to any tree. I reworked 1/4, 2/4 and 3/4 because of some comments from Benjamin Tissoires (mail from 14th September 2016). The patches are submitted to all recipients at the To, CC-Line (e.g. linux-input mailing list). Maybe I can merge 2/4 and 4/4 together. This is all for LED control. 1/4 is for key/input handling and 3/4 is a special FNLock handling where I should use a separate patch. Currently I have no feedback regarding the reworked linux-input specific code. I hope I will get it soon :) Best regards, Dennis

1 0

Re: [alsa-devel] [PATCH 4/4] hda: thinkpad_helper: Add support for hid-lenovo LED control
by Dennis Wassenberg 19 Dec '16

19 Dec '16

Hi Jiri, "led_set_func_hid_lenovo" is set to "hid_lenovo_led_set" +#if IS_ENABLED(CONFIG_HID_LENOVO) +static int hda_fixup_thinkpad_hid_prepare(struct hda_codec *codec) +{ + struct hda_gen_spec *spec = codec->spec; + int ret = 0; + + if (!is_thinkpad(codec)) + return -ENODEV; + if (!led_set_func_hid_lenovo) + led_set_func_hid_lenovo = symbol_request(hid_lenovo_led_set); "hid_lenovo_led_set" is introduced in "[PATCH v2 2/2] hda: thinkpad_helper: Add support for hid-lenovo LED control". This means that "[PATCH 4/4] hda: thinkpad_helper: Add support for hid-lenovo LED control" can only work with applied "[PATCH 1/4] hid-lenovo: Add support for X1 Tablet cover special keys" and "[PATCH v2 2/2] hda: thinkpad_helper: Add support for hid-lenovo LED control". Best regards, Dennis On 19.12.2016 11:09, Jiri Kosina wrote: > On Fri, 9 Dec 2016, Dennis Wassenberg wrote: > >> +#if IS_ENABLED(CONFIG_THINKPAD_ACPI) >> + if (led_set_func_tpacpi) >> + led_set_func_tpacpi(TPACPI_LED_MUTE, !enabled); >> +#endif >> +#if IS_ENABLED(CONFIG_HID_LENOVO) >> + if (led_set_func_hid_lenovo) >> + led_set_func_hid_lenovo(HID_LENOVO_LED_MUTE, !enabled); > > Where does led_set_func_hid_lenovo() come from? I don't see it in Linus' > tree as of today. Is that something else than tpacpi_led_set()? > > Thanks, >

1 0