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

[alsa-devel] [PATCH 0/2] ASoC: SOF: fix undefined references
by Pierre-Louis Bossart 08 May '19

08 May '19

kbuild reported a couple of valid issues with undefined references. It's a bit embarrassing that they were not found earlier since the SOF tree is tracked for each PR... Pierre-Louis Bossart (2): ASoC: SOF: xtensa: fix undefined references ASoC: SOF: core: fix undefined nocodec reference sound/soc/sof/core.c | 10 +++++----- sound/soc/sof/intel/Kconfig | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) -- 2.17.1

3 7

[alsa-devel] [PATCH 0/4] ALSA: line6: Cleanup startup sequences
by Takashi Iwai 08 May '19

08 May '19

Hi, while debugging line6 toneport driver bug, I noticed that other line6 drivers have too complex and fragile startup sequences. This is the result of the rewrites with a single delayed work. Maybe for 5.3, as these are no critical fixes. Takashi === Takashi Iwai (4): ALSA: line6: pod: Rewrite complex timer & work combo with a delayed work ALSA: line6: podhd: Rewrite complex timer & work combo with a delayed work ALSA: line6: variax: Rewrite complex timer & work combo with a delayed work ALSA: line6: Drop superfluous timer helper function sound/usb/line6/driver.c | 11 ----- sound/usb/line6/driver.h | 9 ---- sound/usb/line6/pod.c | 85 ++++++++++++-------------------- sound/usb/line6/podhd.c | 73 +++++---------------------- sound/usb/line6/variax.c | 125 ++++++++++++++--------------------------------- 5 files changed, 78 insertions(+), 225 deletions(-) -- 2.16.4

1 4

[alsa-devel] [PATCH] ALSA: line6: toneport: Fix broken usage of timer for delayed execution
by Takashi Iwai 08 May '19

08 May '19

The line6 toneport driver has code for some delayed initialization, and this hits the kernel Oops because mutex and other sleepable functions are used in the timer callback. Fix the abuse by a delayed work instead so that everything works gracefully. Reported-by: syzbot+a07d0142e74fdd595cfb(a)syzkaller.appspotmail.com Cc: <stable(a)vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/usb/line6/toneport.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c index 19bee725de00..325b07b98b3c 100644 --- a/sound/usb/line6/toneport.c +++ b/sound/usb/line6/toneport.c @@ -54,8 +54,8 @@ struct usb_line6_toneport { /* Firmware version (x 100) */ u8 firmware_version; - /* Timer for delayed PCM startup */ - struct timer_list timer; + /* Work for delayed PCM startup */ + struct delayed_work pcm_work; /* Device type */ enum line6_device_type type; @@ -241,9 +241,10 @@ static int snd_toneport_source_put(struct snd_kcontrol *kcontrol, return 1; } -static void toneport_start_pcm(struct timer_list *t) +static void toneport_start_pcm(struct work_struct *work) { - struct usb_line6_toneport *toneport = from_timer(toneport, t, timer); + struct usb_line6_toneport *toneport = + container_of(work, struct usb_line6_toneport, pcm_work.work); struct usb_line6 *line6 = &toneport->line6; line6_pcm_acquire(line6->line6pcm, LINE6_STREAM_MONITOR, true); @@ -393,7 +394,8 @@ static int toneport_setup(struct usb_line6_toneport *toneport) if (toneport_has_led(toneport)) toneport_update_led(toneport); - mod_timer(&toneport->timer, jiffies + TONEPORT_PCM_DELAY * HZ); + schedule_delayed_work(&toneport->pcm_work, + msecs_to_jiffies(TONEPORT_PCM_DELAY * 1000)); return 0; } @@ -405,7 +407,7 @@ static void line6_toneport_disconnect(struct usb_line6 *line6) struct usb_line6_toneport *toneport = (struct usb_line6_toneport *)line6; - del_timer_sync(&toneport->timer); + cancel_delayed_work_sync(&toneport->pcm_work); if (toneport_has_led(toneport)) toneport_remove_leds(toneport); @@ -422,7 +424,7 @@ static int toneport_init(struct usb_line6 *line6, struct usb_line6_toneport *toneport = (struct usb_line6_toneport *) line6; toneport->type = id->driver_info; - timer_setup(&toneport->timer, toneport_start_pcm, 0); + INIT_DELAYED_WORK(&toneport->pcm_work, toneport_start_pcm); line6->disconnect = line6_toneport_disconnect; -- 2.16.4

1 0

[alsa-devel] [PATCH] ALSA: aica: Fix a long-time build breakage
by Takashi Iwai 08 May '19

08 May '19

The build of aica sound driver has been broken since the timer API conversion and some code rewrite. This patch fixes the breakage by using the common substream field, as well as a bit cleaning up wrt the timer handling in the code. Fixes: d522bb6a105f ("ALSA: sh: aica: Convert timers to use timer_setup()") Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/sh/aica.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/sound/sh/aica.c b/sound/sh/aica.c index e7fef3fce44a..a24e486d9d83 100644 --- a/sound/sh/aica.c +++ b/sound/sh/aica.c @@ -303,7 +303,7 @@ static void aica_period_elapsed(struct timer_list *t) { struct snd_card_aica *dreamcastcard = from_timer(dreamcastcard, t, timer); - struct snd_pcm_substream *substream = dreamcastcard->timer_substream; + struct snd_pcm_substream *substream = dreamcastcard->substream; /*timer function - so cannot sleep */ int play_period; struct snd_pcm_runtime *runtime; @@ -335,13 +335,6 @@ static void spu_begin_dma(struct snd_pcm_substream *substream) dreamcastcard = substream->pcm->private_data; /*get the queue to do the work */ schedule_work(&(dreamcastcard->spu_dma_work)); - /* Timer may already be running */ - if (unlikely(dreamcastcard->timer_substream)) { - mod_timer(&dreamcastcard->timer, jiffies + 4); - return; - } - timer_setup(&dreamcastcard->timer, aica_period_elapsed, 0); - dreamcastcard->timer_substream = substream; mod_timer(&dreamcastcard->timer, jiffies + 4); } @@ -379,8 +372,8 @@ static int snd_aicapcm_pcm_close(struct snd_pcm_substream { struct snd_card_aica *dreamcastcard = substream->pcm->private_data; flush_work(&(dreamcastcard->spu_dma_work)); - if (dreamcastcard->timer_substream) - del_timer(&dreamcastcard->timer); + del_timer(&dreamcastcard->timer); + dreamcastcard->substream = NULL; kfree(dreamcastcard->channel); spu_disable(); return 0; @@ -613,6 +606,7 @@ static int snd_aica_probe(struct platform_device *devptr) "Yamaha AICA Super Intelligent Sound Processor for SEGA Dreamcast"); /* Prepare to use the queue */ INIT_WORK(&(dreamcastcard->spu_dma_work), run_spu_dma); + timer_setup(&dreamcastcard->timer, aica_period_elapsed, 0); /* Load the PCM 'chip' */ err = snd_aicapcmchip(dreamcastcard, 0); if (unlikely(err < 0)) -- 2.16.4

1 0

Re: [alsa-devel] [PATCH 01/16] lib: fix match_string() helper on -1 array size
by Ardelean, Alexandru 08 May '19

08 May '19

On Wed, 2019-05-08 at 14:28 +0300, Alexandru Ardelean wrote: > The documentation the `_match_string()` helper mentions that `n` > should be: > * @n: number of strings in the array or -1 for NULL terminated arrays > > The behavior of the function is different, in the sense that it exits on > the first NULL element in the array, regardless of whether `n` is -1 or a > positive number. > > This patch changes the behavior, to exit the loop when a NULL element is > found and n == -1. Essentially, this aligns the behavior with the > doc-string. > > There are currently many users of `match_string()`, and so, in order to > go > through them, the next patches in the series will focus on doing some > cosmetic changes, which are aimed at grouping the users of > `match_string()`. > This is the duplicate & should be dropped. Sorry for this. > Signed-off-by: Alexandru Ardelean <alexandru.ardelean(a)analog.com> > --- > lib/string.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/lib/string.c b/lib/string.c > index 3ab861c1a857..76edb7bf76cb 100644 > --- a/lib/string.c > +++ b/lib/string.c > @@ -648,8 +648,11 @@ int match_string(const char * const *array, size_t > n, const char *string) > > for (index = 0; index < n; index++) { > item = array[index]; > - if (!item) > + if (!item) { > + if (n != (size_t)-1) > + continue; > break; > + } > if (!strcmp(item, string)) > return index; > }

1 0

[alsa-devel] Fwd: [PATCH] ASoC: SOF: Fix unused variable warnings
by Takashi Iwai 08 May '19

08 May '19

Oops, forgot to add to Cc to alsa-devel ML. Forwarding now. Takashi

2 4

Re: [alsa-devel] [PATCH 1/2] ASoC: Add support for Conexant CX2072X CODEC
by Takashi Iwai 08 May '19

08 May '19

On Mon, 06 May 2019 06:26:25 +0200, Mark Brown wrote: > > On Fri, May 03, 2019 at 09:18:26AM +0200, Takashi Iwai wrote: > > On Fri, 03 May 2019 08:47:29 +0200, > > Mark Brown wrote: > > > On Thu, May 02, 2019 at 09:04:06AM +0200, Takashi Iwai wrote: > > > > Mark Brown wrote: > > > > > > This looks *very* much like board configuration rather than a patch - > > > > > there's no kind of test bit and the comments talk specifically about > > > > > OK, will replace with the straight regmap_multi_reg_write(). > > > > That's probably not addressing the issue, a lot of that stuff just > > > doesn't seem like it should be in some fixed configuration table at all. > > > So what's your alternative suggestion? > > Make it either platform data or runtime controls depending on the > specific thing being configured. Though... > > > > Oh, joy. What's the story here? Do you have a datasheet for the part? > > > Not at all. I just refreshed the already submitted patches since I > > have a laptop with the codec. I tried to contact Conexant, but in > > vain, so I decided to submit the renewed one. > > ...this is not going to help. At the very least it needs to be > abundantly clear to anyone reading the code tht any magic register write > sequences like this are being done the way they are due to a badly > documented part from an unsupportive vendor rather than because it's a > good idea in case someone decides to use the driver as an example. Some > comments or something. For safety the bits that look board specific > like the pinmuxing should ideally be behind DMI checks, though if nobody > else ever bought the CODEC that's probably not so important. Fair enough. I'm going to put more comments on that part. Since this is the only code for this codec on the net and all existing platforms (Intel devices, RPi extensions and else) seem working with it -- i.e. all these depend on the same init sequence more or less, so splitting to platform data won't help much in practice. > > > > The jack detection in ASoC is anyway a bit funky, especially when > > > > involved with PM... > > > > What do you mean here? I'm not aware of any issues and the systems I've > > > worked with seemed robust... > > > There are tons of different ways of implementation for jack controls, > > with different API usages. IOW, no consistency. > > The board registration interfaces do vary as the hardware isn't very > standard so there's a bunch of things you can do for configuration > (though there is a generic interface for the 90% case, the main > limitation these days on using it is Intel's lack of any sensible > firmware interface). I'm not aware of any issues with power management > though? Yeah, I understand the various use cases and coverage for a wide range of hardware implementations, but the lack of the documentation is... not a thing we can be proud of. Regarding the PM, my statement wasn't straight, sorry. The fact is that ASoC manages so many different levels of (a sort of) "PM". We have the Linux device-object system and runtime PMs, and we have BIAS level control. The machine needed the jack handling via ACPI GPIO IRQ, that is outside the codec, which looks rather like a rare case, and I had hard time to look for the right choice of the API usage. > > > > > > +EXPORT_SYMBOL_GPL(snd_soc_cx2072x_get_jack_state); > > > > > > Why is this symbol exported? > > > > > It's called from the machine driver. > > > > snd_soc_jack_add_gpios() is called in the machine driver side, and it > > > > needs the jack_status_check callback that calls this function. > > > > That code shouldn't be in the machine driver, the CODEC driver should > > > request any interrupts it needs itself. > > > The similar things are done on many other Intel SST board drivers. > > The current patch just follows the pattern. > > The only example I can find is the max98095 which it just plain buggy in > this regard, that example shouldn't be followed - it looks like it's > just plain an oversight in review. Frankly the export was done by me > and looks like I was just rushing to fix up a build breakage. This is rather a question how generic the codec driver should be written. I changed the code in v5 patchset to embed the jack_gpio stuff inside the codec driver side rather than the machine driver, so the whole exported functions can be reduced now. But, of course, it slightly gives more implicit assumption about the hardware implementations, too. Though, the existing code seems to have already fixed gpio / pin setups, so the other setups wouldn't have worked, in anyway. > > > No, that's not the question. The question is why there is any > > > initialization at all? > > > Again, no idea. These are likely no default values of the hardware > > registers, and we need to set up some. I *guess* ditto for the > > initial register table in the above, too. > > Or it's just a taste setting someone had for their particular system. > With the earlier register default table an awful lot of it is fairly > obvious board specific configuration, doing things like pinmuxing which > will vary with the board. > > It's unlikely there are *no* default values - you'd kind of have to try > to have a specific register range like this with genuinely floating > values. Given that the code for configuring the EQ was broken to IIRC > never take effect I'd not be 100% surprisd if someone couldn't figure > out why their settings weren't taking effect and just bodged something > directly in the driver. Actually I'm fine to drop the whole EQ stuff that brings lots of black magic. Certainly it'll benefit us for code simplification. Let's see. thanks, Takashi

2 7

[alsa-devel] [PATCH] ASoC: max98357a: release GPIO when component removing
by Tzung-Bi Shih 08 May '19

08 May '19

Component's probe() gets GPIO for "sdmode-gpios" via devm_gpiod_get_optional(). The GPIO binds to the device. When the component get removed but the device does not, the GPIO still binds to the device. Then, when the component be probed again, devm_gpiod_get_optional() returns EBUSY. Release the GPIO proactively when component is removing. Signed-off-by: Tzung-Bi Shih <tzungbi(a)google.com> --- sound/soc/codecs/max98357a.c | 42 ++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/sound/soc/codecs/max98357a.c b/sound/soc/codecs/max98357a.c index d037a3e4d323..b11c6cb81f55 100644 --- a/sound/soc/codecs/max98357a.c +++ b/sound/soc/codecs/max98357a.c @@ -27,24 +27,28 @@ #include <sound/soc-dai.h> #include <sound/soc-dapm.h> +struct max98357a_priv { + struct gpio_desc *sdmode; +}; + static int max98357a_daiops_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) { - struct gpio_desc *sdmode = snd_soc_dai_get_drvdata(dai); + struct max98357a_priv *max98357a = snd_soc_dai_get_drvdata(dai); - if (!sdmode) + if (!max98357a->sdmode) return 0; switch (cmd) { case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - gpiod_set_value(sdmode, 1); + gpiod_set_value(max98357a->sdmode, 1); break; case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - gpiod_set_value(sdmode, 0); + gpiod_set_value(max98357a->sdmode, 0); break; } @@ -61,19 +65,31 @@ static const struct snd_soc_dapm_route max98357a_dapm_routes[] = { static int max98357a_component_probe(struct snd_soc_component *component) { - struct gpio_desc *sdmode; + struct max98357a_priv *max98357a = + snd_soc_component_get_drvdata(component); - sdmode = devm_gpiod_get_optional(component->dev, "sdmode", GPIOD_OUT_LOW); - if (IS_ERR(sdmode)) - return PTR_ERR(sdmode); + max98357a->sdmode = devm_gpiod_get_optional(component->dev, + "sdmode", GPIOD_OUT_LOW); + if (IS_ERR(max98357a->sdmode)) + return PTR_ERR(max98357a->sdmode); - snd_soc_component_set_drvdata(component, sdmode); + snd_soc_component_set_drvdata(component, max98357a); return 0; } +static void max98357a_component_remove(struct snd_soc_component *component) +{ + struct max98357a_priv *max98357a = + snd_soc_component_get_drvdata(component); + + if (max98357a->sdmode) + devm_gpiod_put(component->dev, max98357a->sdmode); +} + static const struct snd_soc_component_driver max98357a_component_driver = { .probe = max98357a_component_probe, + .remove = max98357a_component_remove, .dapm_widgets = max98357a_dapm_widgets, .num_dapm_widgets = ARRAY_SIZE(max98357a_dapm_widgets), .dapm_routes = max98357a_dapm_routes, @@ -112,6 +128,14 @@ static struct snd_soc_dai_driver max98357a_dai_driver = { static int max98357a_platform_probe(struct platform_device *pdev) { + struct max98357a_priv *max98357a; + + max98357a = devm_kzalloc(&pdev->dev, sizeof(*max98357a), GFP_KERNEL); + if (!max98357a) + return -ENOMEM; + + dev_set_drvdata(&pdev->dev, max98357a); + return devm_snd_soc_register_component(&pdev->dev, &max98357a_component_driver, &max98357a_dai_driver, 1); -- 2.21.0.1020.gf2820cf01a-goog

2 3

[alsa-devel] Applied "ASoC: SOF: Fix unused variable warnings" to the asoc tree
by Mark Brown 08 May '19

08 May '19

The patch ASoC: SOF: Fix unused variable warnings 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 2a79e3d342e4b8883ff7fe2cf1b3e9192c26f61a Mon Sep 17 00:00:00 2001 From: Takashi Iwai <tiwai(a)suse.de> Date: Wed, 8 May 2019 10:50:37 +0200 Subject: [PATCH] ASoC: SOF: Fix unused variable warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recent fix for the build fix caused a couple of unused variable compiler warnings when CONFIG_SND_SOC_SOF_NOCODEC isn't set: sound/soc/sof/core.c:263:6: warning: unused variable ‘ret’ [-Wunused-variable] sound/soc/sof/core.c:262:28: warning: unused variable ‘machine’ [-Wunused-variable] Fix them by adding another ifdef. Fixes: ce38a75089f7 ("ASoC: SOF: core: fix undefined nocodec reference") Signed-off-by: Takashi Iwai <tiwai(a)suse.de> Signed-off-by: Mark Brown <broonie(a)kernel.org> --- sound/soc/sof/core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 5ddbfa8f1a28..32105e0fabe8 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -259,8 +259,10 @@ int snd_sof_create_page_table(struct snd_sof_dev *sdev, static int sof_machine_check(struct snd_sof_dev *sdev) { struct snd_sof_pdata *plat_data = sdev->pdata; +#if IS_ENABLED(CONFIG_SND_SOC_SOF_NOCODEC) struct snd_soc_acpi_mach *machine; int ret; +#endif if (plat_data->machine) return 0; -- 2.20.1

1 0

[alsa-devel] [GIT PULL] ASoC fixes for v5.2
by Mark Brown 08 May '19

08 May '19

The following changes since commit ce38a75089f70f6380fc63a5478a7659b4eb3f47: ASoC: SOF: core: fix undefined nocodec reference (2019-05-08 15:42:00 +0900) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git tags/asoc-v5.2-3 for you to fetch changes up to ecc425f06c19a296622c827cde54735ebc5320b8: spi: rockchip: turn down tx dma bursts (2019-05-08 18:04:32 +0900) ---------------------------------------------------------------- ASoC: Even more fixes Yet more stuff that came in through the day. ---------------------------------------------------------------- Emil Renner Berthing (1): spi: rockchip: turn down tx dma bursts Libin Yang (1): ASoC: codec: hdac_hdmi: no checking monitor in hw_params Olivier Moysan (1): ASoC: stm32: i2s: update pcm hardware constraints Tzung-Bi Shih (3): ASoC: mediatek: mt6358: initialize setting when ramping volume ASoC: mediatek: mt6358: save output volume for mixer controls ASoC: mediatek: mt6358: save PGA for mixer control drivers/spi/spi-rockchip.c | 4 +- sound/soc/codecs/hdac_hdmi.c | 13 ----- sound/soc/codecs/mt6358.c | 131 +++++++++++++++++++++++++++---------------- sound/soc/stm/stm32_i2s.c | 3 +- 4 files changed, 87 insertions(+), 64 deletions(-)

1 0