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

[alsa-devel] [PATCH v2 0/2] ASoC: Intel: Add Cometlake PCI IDs
by Evan Green 09 May '19

09 May '19

This small series adds PCI IDs for Cometlake platforms, for a dazzling audio experience. Changes in v2: - Add CML-H ID 0x06c8 (Pierre-Louis) - Add 0x06c8 for CML-H (Pierre-Louis) Evan Green (2): ASoC: SOF: Add Comet Lake PCI IDs ASoC: Intel: Skylake: Add Cometlake PCI IDs sound/soc/intel/Kconfig | 18 +++++++++++++++ sound/soc/intel/skylake/skl-messages.c | 16 +++++++++++++ sound/soc/intel/skylake/skl.c | 10 ++++++++ sound/soc/sof/intel/Kconfig | 32 ++++++++++++++++++++++++++ sound/soc/sof/sof-pci-dev.c | 8 +++++++ 5 files changed, 84 insertions(+) -- 2.20.1

2 10

[alsa-devel] [PATCH 0/8] soundwire: corrections to ACPI and DisCo properties
by Pierre-Louis Bossart 08 May '19

08 May '19

Now that we are done with cleanups, we can start fixing the code with actual semantic or functional changes. This patchset applies on top of everything Vinod and I contributed this week. The fist patch correct a simplifying assumption made earlier. With the IceLake BIOS the existing code is fooled by the presence of a second child device and the namespace walk needs to be filtered. This was not needed on previous generations. The second patch fixes a long-standing misalignment between code and DisCo specification, preventing MIPI DisCo properties from being parsed successfully. The third and fourth patch remove restrictions preventing codec drivers from reading DisCo properties. The fifth patch adds definitions from the SoundWire spec that were missed somehow, but will be very much needed for dynamic bandwidth allocation. The last 3 patches realign the code with the MIPI specification. The existing code exposes properties that don't exist, or exposes them with ambiguous wording. Sticking to the specification helps avoid interpretation issues for integrators, and will make sure the follow-up sysfs support is self-explanatory. Parts of this code was initially written by my Intel colleagues Vinod Koul, Sanyog Kale, Shreyas Nc and Hardik Shah, who are either no longer with Intel or no longer involved in SoundWire development. When relevant, I explictly added a note in commit messages to give them credit for their hard work, but I removed their signed-off-by tags to avoid email bounces and avoid spamming them forever with SoundWire patches. Pierre-Louis Bossart (8): soundwire: intel: filter SoundWire controller device search soundwire: mipi_disco: fix master/link error soundwire: mipi_disco: expose sdw_slave_read_dpn as symbol soundwire: mipi_disco: expose sdw_slave_read_dp0 as symbol soundwire: add port-related definitions soundwire: remove master data port properties soundwire: fix master properties soundwire: rename/clarify MIPI DisCo properties drivers/soundwire/bus.c | 6 +-- drivers/soundwire/intel.c | 11 ++-- drivers/soundwire/intel_init.c | 19 ++++++- drivers/soundwire/mipi_disco.c | 49 +++++++++--------- drivers/soundwire/stream.c | 8 +-- include/linux/soundwire/sdw.h | 94 +++++++++++++++++++++++++++------- 6 files changed, 132 insertions(+), 55 deletions(-) -- 2.17.1

2 16

[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