March 2017 - Alsa-devel - mailman.alsa-project.org

Re: [alsa-devel] [PATCH for-4.11] ASoC: don't dereference NULL pcm_{new, free}
by Kuninori Morimoto 13 Mar '17

13 Mar '17

Hi Brian Thank you for your feedback > There are 4 drivers calling that: > > snd_soc_dummy_probe > rt5514_spi_probe > 2 instances of snd_dmaengine_pcm_register, via rockchip_i2s_probe > > Only the latter two seem to run the assignment here: > > if (platform_drv->pcm_new) > platform->component.pcm_new = snd_soc_platform_drv_pcm_new; > > Both snd_soc_dummy_probe and rt5514_spi_probe find ->pcm_new NULL here. Hmm... The crasher was snd_dmaengine_pcm_register's platform ? This means, in your current kernel, dmaengine platform dosn't call its .pcm_new (= dmaengine_pcm_new) somehow ? I'm wondering why ->pcm_new became NULL which exists on probe timing ? Can you check component and driver by this patch ? This is very rough but enough for debug --------------------- diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 5933851..43da1ec 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -3322,6 +3322,10 @@ static int snd_soc_platform_drv_pcm_new(struct snd_soc_pcm_runtime *rtd) { struct snd_soc_platform *platform = rtd->platform; + printk("-------use name: %s, %p\n", + platform->component.name, + platform->driver); + return platform->driver->pcm_new(rtd); } @@ -3356,8 +3360,12 @@ int snd_soc_add_platform(struct device *dev, struct snd_soc_platform *platform, platform->component.probe = snd_soc_platform_drv_probe; if (platform_drv->remove) platform->component.remove = snd_soc_platform_drv_remove; - if (platform_drv->pcm_new) + if (platform_drv->pcm_new) { + printk("-------add name: %s, %p\n", + platform->component.name, + platform->driver); platform->component.pcm_new = snd_soc_platform_drv_pcm_new; + } if (platform_drv->pcm_free) platform->component.pcm_free = snd_soc_platform_drv_pcm_free; ---------------------

1 0

[alsa-devel] [PATCH] ASoC: cs35l35: trivial fix to indentation
by Colin King 12 Mar '17

12 Mar '17

From: Colin Ian King <colin.king(a)canonical.com> Remove extraneous tab to correct the nesting level indentation Detected by CoverityScan, CID#1416584 ("Nesting level does not match indentation") Signed-off-by: Colin Ian King <colin.king(a)canonical.com> --- sound/soc/codecs/cs35l35.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sound/soc/codecs/cs35l35.c b/sound/soc/codecs/cs35l35.c index 260ed42..43d33f4 100644 --- a/sound/soc/codecs/cs35l35.c +++ b/sound/soc/codecs/cs35l35.c @@ -1370,7 +1370,8 @@ static int cs35l35_i2c_probe(struct i2c_client *i2c_client, for (i = 0; i < ARRAY_SIZE(cs35l35_supplies); i++) cs35l35->supplies[i].supply = cs35l35_supplies[i]; - cs35l35->num_supplies = ARRAY_SIZE(cs35l35_supplies); + + cs35l35->num_supplies = ARRAY_SIZE(cs35l35_supplies); ret = devm_regulator_bulk_get(&i2c_client->dev, cs35l35->num_supplies, -- 2.10.2

2 1

Re: [alsa-devel] [PATCH 01/10] ALSA SoC MAX98927 driver - Initial release
by Rob Herring 12 Mar '17

12 Mar '17

On Fri, Mar 03, 2017 at 11:52:39PM +0900, Ryan Lee wrote: > From: Ryan Lee <RyanS.Lee(a)maximintegrated.com> > > Signed-off-by: Ryan Lee <ryans.lee(a)maximintegrated.com> > --- > > Initial release of ALSA SoC MAX98927 driver. > > .../devicetree/bindings/sound/max98927.txt | 32 + In addition to what Mark said, please split bindings to a separate patch. > sound/soc/codecs/Kconfig | 5 + > sound/soc/codecs/Makefile | 2 + > sound/soc/codecs/max98927.c | 954 +++++++++++++++ > sound/soc/codecs/max98927.h | 1253 ++++++++++++++++++++ > 5 files changed, 2246 insertions(+) > create mode 100755 Documentation/devicetree/bindings/sound/max98927.txt > mode change 100644 => 100755 sound/soc/codecs/Kconfig > mode change 100644 => 100755 sound/soc/codecs/Makefile > create mode 100755 sound/soc/codecs/max98927.c > create mode 100755 sound/soc/codecs/max98927.h These files should not be executable. Rob

1 0

[alsa-devel] bcm2835-analog-audio: Is my driver PWM, ALSA or neither
by Michael Zoran 12 Mar '17

12 Mar '17

I have submitted to ALSA a driver that uses the PWM hardware of the Raspbery PI(bcm2835) as an audio driver. http://mailman.alsa-project.org/pipermail/alsa-devel/2017-March/118652. html The driver is currently mutually exclusive with the standard PWM driver, mostly because the standard PWM interface is not adequate to have an audio driver written on top of it. But as I think more, I see no reason the user needs to chose at boot time if they want traditional PWM or AUDIO. So I'm thinking of some how combining the two drivers. My question: 1. Is my driver a PWM driver that exposes an ALSA interface in additon to PWM? 2. Is my driver a ALSA driver that also exposes PWM. 3. Could my driver potentially go under misc or something new and expose both a PWM and ALSA interface. 4. Perhaps I should have both a PWM driver and a ALSA driver that both talk through a totally new but very small driver that controls access to the PWM hardware. How would the function drivers get loaded in this case? And the $10,000 question of the day is who should I be e-mailing the driver to and which topic branches should the driver be merged through?

1 0

Re: [alsa-devel] [PATCH] Meter causes freeze when closing audio device
by Alistair Buxton 12 Mar '17

12 Mar '17

After adding assert(0); to snd_timer_stop I got the following backtrace: #0 0x00007ffff730d428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54 #1 0x00007ffff730f02a in __GI_abort () at abort.c:89 #2 0x00007ffff7305bd7 in __assert_fail_base (fmt=<optimised out>, assertion=assertion@entry=0x7ffff7b9439e "0", file=file@entry=0x7ffff7b9e8b9 "timer.c", line=line@entry=918, function=function@entry=0x7ffff7b9e9c8 <__PRETTY_FUNCTION__.10913> "snd_timer_stop") at assert.c:92 #3 0x00007ffff7305c82 in __GI___assert_fail (assertion=assertion@entry=0x7ffff7b9439e "0", file=file@entry=0x7ffff7b9e8b9 "timer.c", line=line@entry=918, function=function@entry=0x7ffff7b9e9c8 <__PRETTY_FUNCTION__.10913> "snd_timer_stop") at assert.c:101 #4 0x00007ffff7b64503 in snd_timer_stop (timer=<optimised out>) at timer.c:918 #5 0x00007ffff7b5158d in snd_pcm_dmix_sync_ptr0 (pcm=pcm@entry=0x622de0, slave_hw_ptr=<optimised out>) at pcm_dmix.c:421 #6 0x00007ffff7b51aac in snd_pcm_dmix_status (pcm=0x622de0, status=0x7ffff6af1e80) at pcm_dmix.c:496 #7 0x00007ffff7b18f11 in snd_pcm_status (pcm=pcm@entry=0x622de0, status=status@entry=0x7ffff6af1e80) at pcm.c:977 #8 0x00007ffff7b48bc0 in snd_pcm_meter_thread (data=0x622940) at pcm_meter.c:200 #9 0x00007ffff76a86ba in start_thread (arg=0x7ffff6af2700) at pthread_create.c:333 #10 0x00007ffff73de82d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109 So the meter thread is indirectly stopping the timer while the main thread is trying to poll it. The following patch avoids this issue. Is it the right way to fix it? >From f21cd1ec8245198279cc2ad606041f7f59e8341d Mon Sep 17 00:00:00 2001 From: Alistair Buxton <a.j.buxton(a)gmail.com> Date: Mon, 6 Mar 2017 02:50:35 +0000 Subject: [PATCH] pcm: meter: Put a mutex around slave drain to prevent deadlock The meter thread calls _status on its slave. If this happens while dmix is draining, the timer will be stopped, causing the main thread to get stuck in poll(). --- src/pcm/pcm_meter.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/pcm/pcm_meter.c b/src/pcm/pcm_meter.c index d71af7a..43a25fc 100644 --- a/src/pcm/pcm_meter.c +++ b/src/pcm/pcm_meter.c @@ -324,6 +324,18 @@ static int snd_pcm_meter_start(snd_pcm_t *pcm) return err; } +static int snd_pcm_meter_drain(snd_pcm_t *pcm) +{ + snd_pcm_meter_t *meter = pcm->private_data; + int err; + pthread_mutex_lock(&meter->running_mutex); + err = snd_pcm_drain(meter->gen.slave); + if (err >= 0) + pthread_cond_signal(&meter->running_cond); + pthread_mutex_unlock(&meter->running_mutex); + return err; +} + static snd_pcm_sframes_t snd_pcm_meter_rewind(snd_pcm_t *pcm, snd_pcm_uframes_t frames) { snd_pcm_meter_t *meter = pcm->private_data; @@ -530,7 +542,7 @@ static const snd_pcm_fast_ops_t snd_pcm_meter_fast_ops = { .reset = snd_pcm_meter_reset, .start = snd_pcm_meter_start, .drop = snd_pcm_generic_drop, - .drain = snd_pcm_generic_drain, + .drain = snd_pcm_meter_drain, .pause = snd_pcm_generic_pause, .rewindable = snd_pcm_generic_rewindable, .rewind = snd_pcm_meter_rewind, -- 2.7.4 -- Alistair Buxton a.j.buxton(a)gmail.com

2 1

Re: [alsa-devel] [PATCH for-4.11] ASoC: don't dereference NULL pcm_{new, free}
by Mark Brown 09 Mar '17

09 Mar '17

On Wed, Mar 08, 2017 at 03:18:54PM -0800, Brian Norris wrote: > Not all platform drivers have pcm_{new,free} callbacks. Seen with a > "snd-soc-dummy" codec from sound/soc/rockchip/rk3399_gru_sound.c. > > Resolves an OOPS seen on v4.11-rc1 with Google Kevin (Samsung Chromebook > Plus): > > [ 2.863304] rk3399-gru-sound sound: HiFi <-> ff880000.i2s mapping ok > [ 2.886930] rk3399-gru-sound sound: rt5514-aif1 <-> ff880000.i2s mapping ok Please think hard before including complete backtraces in upstream reports, they are very large and contain almost no useful information relative to their size so often obscure the relevant content in your message. If part of the backtrace is usefully illustrative then it's usually better to pull out the relevant sections.

1 0

[alsa-devel] (no subject)
by Felix Bruns 09 Mar '17

09 Mar '17

Hello, I am working on embedded device (using an i.MX6 MCU) which has an external amplifier for sound amplification. Unfortunately I could not find information if and how the gain of hardware amplifiers are controlled by ALSA. Can somebody clarify or point me to an example? Thanks and kind regards, Felix Bruns

1 0

Re: [alsa-devel] [PATCH for-4.11] ASoC: don't dereference NULL pcm_{new, free}
by Kuninori Morimoto 09 Mar '17

09 Mar '17

Hi Brian > > It is a littlle bit strange for me. > > Yes, and honestly I'm a little confused by the inheritance in this > framework. Yes, I agree :) This is 1st prepare for future ALSA SoC framework cleanup It is Lars-Peter's idea > I have a feeling you're checking the wrong thing below for this case. If so, we should fix this "wrong thing" ? > All I know is that I'm definitely hitting a NULL > platform->driver->pcm_new callback, and that either reverting your patch > or applying the patch I just sent fixes it. I want to know why this happen. Can you show me which driver is calling snd_soc_add_platform() in your case ?

1 0

Re: [alsa-devel] [PATCH for-4.11] ASoC: don't dereference NULL pcm_{new, free}
by Kuninori Morimoto 09 Mar '17

09 Mar '17

Hi Brian Thank you for your patch > Not all platform drivers have pcm_{new,free} callbacks. Seen with a > "snd-soc-dummy" codec from sound/soc/rockchip/rk3399_gru_sound.c. (snip) > Fixes: 99b04f4c4051 ("ASoC: add Component level pcm_new/pcm_free") > Signed-off-by: Brian Norris <briannorris(a)chromium.org> > --- > I'm really not that familiar with this subsystem... but this does fix the > crash seen here. > > sound/soc/soc-core.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > index 6dca408faae3..2722bb0c5573 100644 > --- a/sound/soc/soc-core.c > +++ b/sound/soc/soc-core.c > @@ -3326,7 +3326,10 @@ static int snd_soc_platform_drv_pcm_new(struct snd_soc_pcm_runtime *rtd) > { > struct snd_soc_platform *platform = rtd->platform; > > - return platform->driver->pcm_new(rtd); > + if (platform->driver->pcm_new) > + return platform->driver->pcm_new(rtd); > + else > + return 0; > } > > static void snd_soc_platform_drv_pcm_free(struct snd_pcm *pcm) > @@ -3334,7 +3337,8 @@ static void snd_soc_platform_drv_pcm_free(struct snd_pcm *pcm) > struct snd_soc_pcm_runtime *rtd = pcm->private_data; > struct snd_soc_platform *platform = rtd->platform; > > - platform->driver->pcm_free(pcm); > + if (platform->driver->pcm_free) > + platform->driver->pcm_free(pcm); > } It is a littlle bit strange for me. commit 99b04f4c4051 has below code. This means, if platform doesn't have pcm_new/free callback, component doesn't have snd_soc_platform_drv_pcm_new/free. But your case, platform doesn't have pcm_new/free, but component had it ? ... @@ -3181,6 +3198,10 @@ int snd_soc_add_platform(struct device *dev, struct snd_soc_platform *platform, platform->component.probe = snd_soc_platform_drv_probe; if (platform_drv->remove) platform->component.remove = snd_soc_platform_drv_remove; + if (platform_drv->pcm_new) + platform->component.pcm_new = snd_soc_platform_drv_pcm_new; + if (platform_drv->pcm_free) + platform->component.pcm_free = snd_soc_platform_drv_pcm_free; #ifdef CONFIG_DEBUG_FS platform->component.debugfs_prefix = "platform";

1 0

Re: [alsa-devel] [RFC PATCH] sound: bcm2835-analog-audio: Add driver for bcm2835(Raspberry PI) headphone jack.
by Eric Anholt 09 Mar '17

09 Mar '17

Michael Zoran <mzoran(a)crowfest.net> writes: > Someone on the internet mentioned that if we have a HDMI Audio driver, why not > have a driver for the headphone/analog jack on the Raspberry PI that doesn't > use the firmware. So I quickly make up a prototype that isn't submit quality > yet, but is very functional. The idea is to like the firmware, just use the > PWM of the bcm2835 to generate the audio. > > Questions: > > 1. Is this something that people would want in the upstream kernel and > would be seriously considered for submission? I'm certainly interested -- the more we can take over from the closed source stack, the better Linux on the Raspberry Pi will be. Make sure you use standard DT properties for describing the DMA channel, add your new compatible string to pwm-bcm2835.txt, and document that that compat string requires the DMA properties. > 2. Is it a complete deal breaker that the driver is a regular ALSA driver > instead of a ASoC driver? I don't think so. I think the goal of ASoC is to provide utilities that make your SoC ALSA drivers smaller.

1 0