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

Re: [alsa-devel] [PATCH -next] ASoC: samsung: i2s: Fix return value check in i2s_create_secondary_device()
by Krzysztof Kozlowski 18 Feb '19

18 Feb '19

On Sat, 16 Feb 2019 at 02:07, Wei Yongjun <weiyongjun1(a)huawei.com> wrote: > > In case of error, the function platform_device_register_simple() returns > ERR_PTR() and never returns NULL. The NULL test in the return value > check should be replaced with IS_ERR(). > > Fixes: 7196c64c7d0c ("ASoC: samsung: i2s: Restore support for the secondary PCM") > Signed-off-by: Wei Yongjun <weiyongjun1(a)huawei.com> > --- > sound/soc/samsung/i2s.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Krzysztof Kozlowski <krzk(a)kernel.org> Best regards, Krzysztof

1 0

Re: [alsa-devel] [PATCH] ASoC: simple-card: fixup refcount_t underflow
by Daniel Baluta 18 Feb '19

18 Feb '19

On Fri, Feb 15, 2019 at 8:43 PM Takashi Iwai <tiwai(a)suse.de> wrote: > > On Fri, 15 Feb 2019 18:31:08 +0100, > Mark Brown wrote: > > > > On Fri, Feb 15, 2019 at 06:21:04PM +0100, Takashi Iwai wrote: > > > > > Actually it's not all. As mentioned in my earlier post, there are > > > more of refcount unbalance in the code. Lots of them, really. > > > > > Below is a patch I've fixed quickly over the whole scanning the sound > > > tree. Some might be incorrect, so more detailed review for each piece > > > of change is needed... > > > > Extra references aren't such a problem for practical systems since we > > don't actually ever free any of the DT (there's no mainline way to use > > overlays yet...), that's how none of this stuff ever gets noticed. I'm > > not surprised there's loads of missing frees TBH. In contrast if we've > > got extra frees they will get noticed and cause problems. > > Yeah, the rest are just leaks, and we have already tons of them :) > > The point is that the commit in question introduced multiple such > issues, so they should be addressed as well. > > But, the biggest problem is that OF api is very hard to use right... Hello Mark/Takashi, Just sent this https://lkml.org/lkml/2019/2/16/52 to fix the problem that was bugging me and to update the patch from Kuninori. Feel free to send a patch for the rest of the refcount issues :). Kuninori, if you consider necessary please add your Signed-off-by to the patch as it is mostly your work but I didn't know exactly how to give you credit for it :(. thanks, Daniel.

2 1

Re: [alsa-devel] [PATCH] ASoC: simple-card: fixup refcount_t underflow
by Kuninori Morimoto 18 Feb '19

18 Feb '19

Hi Takashi-san > > commit da215354eb55c ("ASoC: simple-card: merge simple-scu-card") > > merged simple-card and simple-scu-card. Then it had refcount > > underflow bug. This patch fixup it. > > We will get below error without this patch. > > > > OF: ERROR: Bad of_node_put() on /sound > > CPU: 3 PID: 237 Comm: kworker/3:1 Not tainted 5.0.0-rc6+ #1514 > > Hardware name: Renesas H3ULCB Kingfisher board based on r8a7795 ES2.0+ (DT) > > Workqueue: events deferred_probe_work_func > > Call trace: > > dump_backtrace+0x0/0x150 > > show_stack+0x24/0x30 > > dump_stack+0xb0/0xec > > of_node_release+0xd0/0xd8 > > kobject_put+0x74/0xe8 > > of_node_put+0x24/0x30 > > __of_get_next_child+0x50/0x70 > > of_get_next_child+0x40/0x68 > > asoc_simple_card_probe+0x604/0x730 > > platform_drv_probe+0x58/0xa8 > > ... > > Reported-by: Vicente Bergas <vicencb(a)gmail.com> > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx(a)renesas.com> > > Please don't forget to put Fixes tag for a regression fix. Oops, indeed. Thanks. will add it on v2 patch. > And, looking at the code in 5.0-rc, it seems still leaving the > reference in some condition. The 5.0-rc code corrected your patch > looks like: > > node = of_get_child_by_name(top, PREFIX "dai-link"); > if (!node) { > node = of_node_get(top); > loop = 0; > } > > do { > .... > node = of_get_next_child(top, node); > } while (loop && node); > > So when loop=0, it aborts the loop at the first iteration. If node is > non-NULL at that point, it still keeps the refcount taken in > of_get_next_child(). > > That is, you'd need to call of_node_put(node) after the loop. Ah.. yes.. > And, this leads me checking the whole code, and I'm afraid that we > have many similar unblanced refcounts. For example, a tricky one is > for_each_child_of_node() loop. When you abort at the middle of the > loop, you'd need to unreference the remaining node. But we return or > abort immediately without unreferencing in many places. > > Hrm... Yes, I had noticed this refcounts issue. The code can be very confusable for this refcounts... Best regards --- Kuninori Morimoto

1 0

Re: [alsa-devel] [PATCH] ASoC: fsl_esai: fix register setting issue in RIGHT_J mode
by Fabio Estevam 16 Feb '19

16 Feb '19

Hi Shengjiu, On Fri, Feb 15, 2019 at 9:04 AM S.j. Wang <shengjiu.wang(a)nxp.com> wrote: > > The ESAI_xCR_xWA is xCR's bit, not the xCCR's bit, driver set it to > wrong register, correct it. > > Signed-off-by: Shengjiu Wang <shengjiu.wang(a)nxp.com> Reviewed-by: Fabio Estevam <festevam(a)gmail.com> Please add a Fixes tag and Cc stable. Also, it seems that Mark Brown is not on Cc. Please Cc him in v2. Thanks

1 0

Re: [alsa-devel] Saffire Pro 26 IO Restarting Issue
by Takashi Sakamoto 15 Feb '19

15 Feb '19

Hi, On Fri, Feb 15, 2019 at 12:49:19PM +0000, ackolonges fds wrote: > Hi All, > > I was wondering if there might be anyone on here that could give me any > pointers in getting my Focusrite Saffire Pro 26 i/o interface working with > Linux via FFADO? > > It's a new desktop PC (Ryzen 5 2600, 8GB RAM), dual booting Windows 10 and > Ubuntu 18.10, with a PCI Express, Texas Instruments XIO2213 Firewire card. > > I have tested this Saffire interface successfully on a MacBook Pro (it was > plug and play), and also on this very desktop computer in Windows 10. > However, when the interface is connected in Ubuntu 18.10, it repeatedly > and periodically resets itself about 10-15 seconds after booting up, in a > loop. If I run ffado-dbus-server while it's booting, it connects > successfully to the interface, and quickly running FFADO mixer afterwards > lets me see the controls available, for a short time until the interface > resets itself, cutting all the connections. > > Has anybody here ever heard of this kind of issue before? Or perhaps > someone may have some ideas that might aid in troubleshooting the issue? > > Thanks so much for any advice you may have, In case of Focusrite Saffire Pro I/O 10 and 26, a combination of ALSA bebob driver and pulseaudio brings this problem due to quirk of these two models reported in bugzilla.kernel.org[1]. I've already committed a solution and it was merged to pulseaudio[2], however it has never included into any releases of pulseaudio at present. Therefore you can see this issue on any desktop environment in released distribution such as the latest Ubuntu. You can avoid this issue by two handy solutions at a quick glance: 1. suppress pulseaudio to handle your device 2. suppress your system to load ALSA bebob driver For solution 1, it's the best way to edit 90-pulseaudio.rules in your system. $ systemctl --user stop pulseaudio.service pulseaudio.socket $ sudo nano /lib/udev/rules.d/90-pulseaudio.rules (modify the above file according to my patch[2].) $ systemctl --user start pulseaudio.service pulseaudio.socket For solution 2, it's the easiest way to add ALSA bebob driver to blacklist. $ sudo nano /etc/modprobe.d/blacklist-bebob.conf (add entry of 'blacklist snd-bebob') (then unplug and plugin your device) Sorry for the inconvenience. [1] https://bugzilla.kernel.org/show_bug.cgi?id=199365 [2] https://gitlab.freedesktop.org/pulseaudio/pulseaudio/commit/37358e42c49abe3… Regards Takashi Sakamoto

1 0

Re: [alsa-devel] [PATCH v5 0/3] Add NXP AUDMIX device and machine drivers
by Nicolin Chen 15 Feb '19

15 Feb '19

On Fri, Feb 15, 2019 at 02:01:32PM +0000, Viorel Suman wrote: > The patchset adds NXP Audio Mixer (AUDMIX) device and machine > drivers and related DT bindings documentation. For this series, Acked-by: Nicolin Chen <nicoleotsuka(a)gmail.com> And Rob gave his at the previous version already. Thanks. > Changes since V4: > 1. Removed "model" attribute from device driver DT bindings documentation > as suggested by Nicolin. > > Changes since V3: > 1. Removed machine driver DT bindings documentation. > 2. Trigger machine driver probe from device driver as suggested by Nicolin. > > Changes since V2: > 1. Moved "dais" node from machine driver DTS node to device driver DTS node > as suggested by Rob. > > Changes since V1: > 1. Original patch split into distinct patches for the device driver and > DT binding documentation. > 2. Replaced AMIX with AUDMIX in both code and file names as it looks more > RM-compliant. > 3. Removed polarity control from CPU DAI driver as suggested by Nicolin. > 4. Added machine driver and related DT binding documentation. > > Viorel Suman (3): > ASoC: fsl: Add Audio Mixer CPU DAI driver > ASoC: add fsl_audmix DT binding documentation > ASoC: fsl: Add Audio Mixer machine driver > > .../devicetree/bindings/sound/fsl,audmix.txt | 50 ++ > sound/soc/fsl/Kconfig | 16 + > sound/soc/fsl/Makefile | 5 + > sound/soc/fsl/fsl_audmix.c | 578 +++++++++++++++++++++ > sound/soc/fsl/fsl_audmix.h | 102 ++++ > sound/soc/fsl/imx-audmix.c | 327 ++++++++++++ > 6 files changed, 1078 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/fsl,audmix.txt > create mode 100644 sound/soc/fsl/fsl_audmix.c > create mode 100644 sound/soc/fsl/fsl_audmix.h > create mode 100644 sound/soc/fsl/imx-audmix.c > > -- > 2.7.4 >

1 0

Re: [alsa-devel] [PATCH] ASoC: fsl_esai: fix register setting issue in RIGHT_J mode
by Nicolin Chen 15 Feb '19

15 Feb '19

On Fri, Feb 15, 2019 at 11:04:38AM +0000, S.j. Wang wrote: > The ESAI_xCR_xWA is xCR's bit, not the xCCR's bit, driver set it to > wrong register, correct it. > > Signed-off-by: Shengjiu Wang <shengjiu.wang(a)nxp.com> Would need this for stable kernel too. Ackedy-by: Nicolin Chen <nicoleotsuka(a)gmail.com> Thanks. > --- > sound/soc/fsl/fsl_esai.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c > index 57b484768a58..afe67c865330 100644 > --- a/sound/soc/fsl/fsl_esai.c > +++ b/sound/soc/fsl/fsl_esai.c > @@ -398,7 +398,8 @@ static int fsl_esai_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt) > break; > case SND_SOC_DAIFMT_RIGHT_J: > /* Data on rising edge of bclk, frame high, right aligned */ > - xccr |= ESAI_xCCR_xCKP | ESAI_xCCR_xHCKP | ESAI_xCR_xWA; > + xccr |= ESAI_xCCR_xCKP | ESAI_xCCR_xHCKP; > + xcr |= ESAI_xCR_xWA; > break; > case SND_SOC_DAIFMT_DSP_A: > /* Data on rising edge of bclk, frame high, 1clk before data */ > @@ -455,12 +456,12 @@ static int fsl_esai_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt) > return -EINVAL; > } > > - mask = ESAI_xCR_xFSL | ESAI_xCR_xFSR; > + mask = ESAI_xCR_xFSL | ESAI_xCR_xFSR | ESAI_xCR_xWA; > regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR, mask, xcr); > regmap_update_bits(esai_priv->regmap, REG_ESAI_RCR, mask, xcr); > > mask = ESAI_xCCR_xCKP | ESAI_xCCR_xHCKP | ESAI_xCCR_xFSP | > - ESAI_xCCR_xFSD | ESAI_xCCR_xCKD | ESAI_xCR_xWA; > + ESAI_xCCR_xFSD | ESAI_xCCR_xCKD; > regmap_update_bits(esai_priv->regmap, REG_ESAI_TCCR, mask, xccr); > regmap_update_bits(esai_priv->regmap, REG_ESAI_RCCR, mask, xccr); > > -- > 1.9.1 >

1 0

[alsa-devel] Applied "ASoC: dmaengine: Remove unused SND_DMAENGINE_PCM_FLAG_CUSTOM_CHANNEL_NAME flag" to the asoc tree
by Mark Brown 15 Feb '19

15 Feb '19

The patch ASoC: dmaengine: Remove unused SND_DMAENGINE_PCM_FLAG_CUSTOM_CHANNEL_NAME flag has been applied to the asoc tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From 76d9c68b360f852e784170f10cb431e4713c7d0b Mon Sep 17 00:00:00 2001 From: Sylwester Nawrocki <s.nawrocki(a)samsung.com> Date: Thu, 14 Feb 2019 16:45:55 +0100 Subject: [PATCH] ASoC: dmaengine: Remove unused SND_DMAENGINE_PCM_FLAG_CUSTOM_CHANNEL_NAME flag There is now no users of this flag so remove it together with related code. The chan_name field of snd_dmaengine_dai_dma_data data structure is not removed as it is still in use by the PXA platform. Signed-off-by: Sylwester Nawrocki <s.nawrocki(a)samsung.com> Acked-by: Krzysztof Kozlowski <krzk(a)kernel.org> Signed-off-by: Mark Brown <broonie(a)kernel.org> --- include/sound/dmaengine_pcm.h | 4 ---- sound/soc/soc-generic-dmaengine-pcm.c | 21 ++++----------------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h index 2c4cfaa135a6..c679f6116580 100644 --- a/include/sound/dmaengine_pcm.h +++ b/include/sound/dmaengine_pcm.h @@ -99,10 +99,6 @@ void snd_dmaengine_pcm_set_config_from_dai_data( * playback. */ #define SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX BIT(3) -/* - * The PCM streams have custom channel names specified. - */ -#define SND_DMAENGINE_PCM_FLAG_CUSTOM_CHANNEL_NAME BIT(4) /** * struct snd_dmaengine_pcm_config - Configuration data for dmaengine based PCM diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c index 1b44e363c50c..f1ab6285a085 100644 --- a/sound/soc/soc-generic-dmaengine-pcm.c +++ b/sound/soc/soc-generic-dmaengine-pcm.c @@ -265,7 +265,6 @@ static int dmaengine_pcm_new(struct snd_soc_pcm_runtime *rtd) struct dmaengine_pcm *pcm = soc_component_to_pcm(component); const struct snd_dmaengine_pcm_config *config = pcm->config; struct device *dev = component->dev; - struct snd_dmaengine_dai_dma_data *dma_data; struct snd_pcm_substream *substream; size_t prealloc_buffer_size; size_t max_buffer_size; @@ -285,19 +284,9 @@ static int dmaengine_pcm_new(struct snd_soc_pcm_runtime *rtd) if (!substream) continue; - dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream); - - if (!pcm->chan[i] && - ((pcm->flags & SND_DMAENGINE_PCM_FLAG_CUSTOM_CHANNEL_NAME) || - (config && config->chan_names[i]))) { - const char *chan_name = dma_data->chan_name; - - if (config && config->chan_names[i]) - chan_name = config->chan_names[i]; - + if (!pcm->chan[i] && config && config->chan_names[i]) pcm->chan[i] = dma_request_slave_channel(dev, - chan_name); - } + config->chan_names[i]); if (!pcm->chan[i] && (pcm->flags & SND_DMAENGINE_PCM_FLAG_COMPAT)) { pcm->chan[i] = dmaengine_pcm_compat_request_channel(rtd, @@ -420,10 +409,8 @@ static int dmaengine_pcm_request_chan_of(struct dmaengine_pcm *pcm, const char *name; struct dma_chan *chan; - if ((pcm->flags & (SND_DMAENGINE_PCM_FLAG_NO_DT | - SND_DMAENGINE_PCM_FLAG_CUSTOM_CHANNEL_NAME)) || - (!dev->of_node && !(config && config->dma_dev && - config->dma_dev->of_node))) + if ((pcm->flags & SND_DMAENGINE_PCM_FLAG_NO_DT) || (!dev->of_node && + !(config && config->dma_dev && config->dma_dev->of_node))) return 0; if (config && config->dma_dev) { -- 2.20.1

1 0

[alsa-devel] [PATCH] ASoC: samsung: i2s: Prevent potential NULL platform data dereference
by Sylwester Nawrocki 15 Feb '19

15 Feb '19

When np is NULL i2s_pdata could also be NULL but i2s_pdata is now being dereferenced without proper check. Fix this and shorten the error message so we don't exceed 80 characters limit. Reported-by: Dan Carpenter <dan.carpenter(a)oracle.com> Signed-off-by: Sylwester Nawrocki <s.nawrocki(a)samsung.com> --- sound/soc/samsung/i2s.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c index 03fff1c657be..02472f576e17 100644 --- a/sound/soc/samsung/i2s.c +++ b/sound/soc/samsung/i2s.c @@ -1369,7 +1369,7 @@ static int samsung_i2s_probe(struct platform_device *pdev) struct i2s_dai *pri_dai, *sec_dai = NULL; struct s3c_audio_pdata *i2s_pdata = pdev->dev.platform_data; struct resource *res; - u32 regs_base, quirks = 0, idma_addr = 0; + u32 regs_base, idma_addr = 0; struct device_node *np = pdev->dev.of_node; const struct samsung_i2s_dai_data *i2s_dai_data; int num_dais, ret; @@ -1389,11 +1389,19 @@ static int samsung_i2s_probe(struct platform_device *pdev) if (!priv) return -ENOMEM; - quirks = np ? i2s_dai_data->quirks : i2s_pdata->type.quirks; - num_dais = (quirks & QUIRK_SEC_DAI) ? 2 : 1; + if (np) { + priv->quirks = i2s_dai_data->quirks; + } else { + if (!i2s_pdata) { + dev_err(&pdev->dev, "Missing platform data\n"); + return -EINVAL; + } + priv->quirks = i2s_pdata->type.quirks; + } + + num_dais = (priv->quirks & QUIRK_SEC_DAI) ? 2 : 1; priv->pdev = pdev; priv->variant_regs = i2s_dai_data->i2s_variant_regs; - priv->quirks = quirks; ret = i2s_alloc_dais(priv, i2s_dai_data, num_dais); if (ret < 0) @@ -1405,11 +1413,6 @@ static int samsung_i2s_probe(struct platform_device *pdev) spin_lock_init(&priv->pcm_lock); if (!np) { - if (i2s_pdata == NULL) { - dev_err(&pdev->dev, "Can't work without s3c_audio_pdata\n"); - return -EINVAL; - } - pri_dai->dma_playback.filter_data = i2s_pdata->dma_playback; pri_dai->dma_capture.filter_data = i2s_pdata->dma_capture; pri_dai->filter = i2s_pdata->dma_filter; @@ -1418,7 +1421,7 @@ static int samsung_i2s_probe(struct platform_device *pdev) } else { if (of_property_read_u32(np, "samsung,idma-addr", &idma_addr)) { - if (quirks & QUIRK_SUPPORTS_IDMA) { + if (priv->quirks & QUIRK_SUPPORTS_IDMA) { dev_info(&pdev->dev, "idma address is not"\ "specified"); } @@ -1451,7 +1454,7 @@ static int samsung_i2s_probe(struct platform_device *pdev) pri_dai->dma_capture.addr_width = 4; pri_dai->priv = priv; - if (quirks & QUIRK_PRI_6CHAN) + if (priv->quirks & QUIRK_PRI_6CHAN) pri_dai->drv->playback.channels_max = 6; ret = samsung_asoc_dma_platform_register(&pdev->dev, pri_dai->filter, @@ -1459,7 +1462,7 @@ static int samsung_i2s_probe(struct platform_device *pdev) if (ret < 0) goto err_disable_clk; - if (quirks & QUIRK_SEC_DAI) { + if (priv->quirks & QUIRK_SEC_DAI) { sec_dai = &priv->dai[SAMSUNG_I2S_ID_SECONDARY - 1]; sec_dai->dma_playback.addr = regs_base + I2STXDS; -- 2.17.1

3 2

Re: [alsa-devel] [PATCH] ASoC: simple-card: fixup refcount_t underflow
by Mark Brown 15 Feb '19

15 Feb '19

On Fri, Feb 15, 2019 at 06:21:04PM +0100, Takashi Iwai wrote: > Actually it's not all. As mentioned in my earlier post, there are > more of refcount unbalance in the code. Lots of them, really. > Below is a patch I've fixed quickly over the whole scanning the sound > tree. Some might be incorrect, so more detailed review for each piece > of change is needed... Extra references aren't such a problem for practical systems since we don't actually ever free any of the DT (there's no mainline way to use overlays yet...), that's how none of this stuff ever gets noticed. I'm not surprised there's loads of missing frees TBH. In contrast if we've got extra frees they will get noticed and cause problems.

1 0