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

[PATCH] ALSA: usb-audio: Fix recursive locking on XRUN
by John Keeping 20 Mar '23

20 Mar '23

snd_usb_queue_pending_output_urbs() may be called from snd_pcm_ops::ack() which means the PCM stream is locked. For the normal case where the call back into the PCM core is via prepare_output_urb() the "_under_stream_lock" variant of snd_pcm_period_elapsed() is called, but when an error occurs and the stream is stopped as XRUN then snd_pcm_xrun() tries to recursively lock the stream which results in deadlock. Follow the example of snd_pcm_period_elapsed() by adding snd_pcm_xrun_under_stream_lock() and use this when the PCM substream lock is already held. Signed-off-by: John Keeping <john(a)metanate.com> --- include/sound/pcm.h | 1 + sound/core/pcm_native.c | 28 ++++++++++++++++++++++++---- sound/usb/endpoint.c | 18 +++++++++++------- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 27040b472a4f..98551907453a 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -571,6 +571,7 @@ int snd_pcm_status64(struct snd_pcm_substream *substream, int snd_pcm_start(struct snd_pcm_substream *substream); int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t status); int snd_pcm_drain_done(struct snd_pcm_substream *substream); +int snd_pcm_stop_xrun_under_stream_lock(struct snd_pcm_substream *substream); int snd_pcm_stop_xrun(struct snd_pcm_substream *substream); #ifdef CONFIG_PM int snd_pcm_suspend_all(struct snd_pcm *pcm); diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 331380c2438b..617f5dc74df0 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1559,24 +1559,44 @@ int snd_pcm_drain_done(struct snd_pcm_substream *substream) SNDRV_PCM_STATE_SETUP); } +/** + * snd_pcm_stop_xrun_under_stream_lock - stop the running stream as XRUN under the lock of + * the PCM substream. + * @substream: the PCM substream instance + * + * This stops the given running substream (and all linked substreams) as XRUN. + * This function assumes that the substream lock is already held. + * + * Return: Zero if successful, or a negative error core. + */ +int snd_pcm_stop_xrun_under_stream_lock(struct snd_pcm_substream *substream) +{ + if (substream->runtime && snd_pcm_running(substream)) + __snd_pcm_xrun(substream); + + return 0; +} + /** * snd_pcm_stop_xrun - stop the running streams as XRUN * @substream: the PCM substream instance * + * This function is similar to ``snd_pcm_stop_xrun_under_stream_lock()`` except that it + * acquires the substream lock itself. + * * This stops the given running substream (and all linked substreams) as XRUN. - * Unlike snd_pcm_stop(), this function takes the substream lock by itself. * * Return: Zero if successful, or a negative error code. */ int snd_pcm_stop_xrun(struct snd_pcm_substream *substream) { unsigned long flags; + int ret; snd_pcm_stream_lock_irqsave(substream, flags); - if (substream->runtime && snd_pcm_running(substream)) - __snd_pcm_xrun(substream); + ret = snd_pcm_stop_xrun_under_stream_lock(substream); snd_pcm_stream_unlock_irqrestore(substream, flags); - return 0; + return ret; } EXPORT_SYMBOL_GPL(snd_pcm_stop_xrun); diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 1e0af1179ca8..83a6b6d41374 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -400,13 +400,17 @@ static int prepare_inbound_urb(struct snd_usb_endpoint *ep, } /* notify an error as XRUN to the assigned PCM data substream */ -static void notify_xrun(struct snd_usb_endpoint *ep) +static void notify_xrun(struct snd_usb_endpoint *ep, bool in_stream_lock) { struct snd_usb_substream *data_subs; data_subs = READ_ONCE(ep->data_subs); - if (data_subs && data_subs->pcm_substream) - snd_pcm_stop_xrun(data_subs->pcm_substream); + if (data_subs && data_subs->pcm_substream) { + if (in_stream_lock) + snd_pcm_stop_xrun_under_stream_lock(data_subs->pcm_substream); + else + snd_pcm_stop_xrun(data_subs->pcm_substream); + } } static struct snd_usb_packet_info * @@ -498,7 +502,7 @@ void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep, if (err == -EAGAIN) push_back_to_ready_list(ep, ctx); else - notify_xrun(ep); + notify_xrun(ep, in_stream_lock); return; } @@ -507,7 +511,7 @@ void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep, usb_audio_err(ep->chip, "Unable to submit urb #%d: %d at %s\n", ctx->index, err, __func__); - notify_xrun(ep); + notify_xrun(ep, in_stream_lock); return; } @@ -574,7 +578,7 @@ static void snd_complete_urb(struct urb *urb) return; usb_audio_err(ep->chip, "cannot submit urb (err = %d)\n", err); - notify_xrun(ep); + notify_xrun(ep, false); exit_clear: clear_bit(ctx->index, &ep->active_mask); @@ -1762,7 +1766,7 @@ static void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep, usb_audio_err(ep->chip, "next package FIFO overflow EP 0x%x\n", ep->ep_num); - notify_xrun(ep); + notify_xrun(ep, false); return; } -- 2.40.0

4 9

[PATCH] ALSA: hda/conexant: Partial revert of a quirk for Lenovo
by Takashi Iwai 20 Mar '23

20 Mar '23

The recent commit f83bb2592482 ("ALSA: hda/conexant: Add quirk for LENOVO 20149 Notebook model") introduced a quirk for the device with 17aa:3977, but this caused a regression on another model (Lenovo Ideadpad U31) with the very same PCI SSID. And, through skimming over the net, it seems that this PCI SSID is used for multiple different models, so it's no good idea to apply the quirk with the SSID. Although we may take a different ID check (e.g. the codec SSID instead of the PCI SSID), unfortunately, the original patch author couldn't identify the hardware details any longer as the machine was returned, and we can't develop the further proper fix. In this patch, instead, we partially revert the change so that the quirk won't be applied as default for addressing the regression. Meanwhile, the quirk function itself is kept, and it's now made to be applicable via the explicit model=lenovo-20149 option. Fixes: f83bb2592482 ("ALSA: hda/conexant: Add quirk for LENOVO 20149 Notebook model") Reported-by: Jetro Jormalainen <jje-lxkl(a)jetro.fi> Cc: <stable(a)vger.kernel.org> Link: https://lore.kernel.org/r/20230308215009.4d3e58a6@mopti Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/pci/hda/patch_conexant.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c index 75e1d00074b9..a889cccdd607 100644 --- a/sound/pci/hda/patch_conexant.c +++ b/sound/pci/hda/patch_conexant.c @@ -980,7 +980,10 @@ static const struct snd_pci_quirk cxt5066_fixups[] = { SND_PCI_QUIRK(0x17aa, 0x3905, "Lenovo G50-30", CXT_FIXUP_STEREO_DMIC), SND_PCI_QUIRK(0x17aa, 0x390b, "Lenovo G50-80", CXT_FIXUP_STEREO_DMIC), SND_PCI_QUIRK(0x17aa, 0x3975, "Lenovo U300s", CXT_FIXUP_STEREO_DMIC), - SND_PCI_QUIRK(0x17aa, 0x3977, "Lenovo IdeaPad U310", CXT_PINCFG_LENOVO_NOTEBOOK), + /* NOTE: we'd need to extend the quirk for 17aa:3977 as the same + * PCI SSID is used on multiple Lenovo models + */ + SND_PCI_QUIRK(0x17aa, 0x3977, "Lenovo IdeaPad U310", CXT_FIXUP_STEREO_DMIC), SND_PCI_QUIRK(0x17aa, 0x3978, "Lenovo G50-70", CXT_FIXUP_STEREO_DMIC), SND_PCI_QUIRK(0x17aa, 0x397b, "Lenovo S205", CXT_FIXUP_STEREO_DMIC), SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", CXT_FIXUP_THINKPAD_ACPI), @@ -1003,6 +1006,7 @@ static const struct hda_model_fixup cxt5066_fixup_models[] = { { .id = CXT_FIXUP_MUTE_LED_GPIO, .name = "mute-led-gpio" }, { .id = CXT_FIXUP_HP_ZBOOK_MUTE_LED, .name = "hp-zbook-mute-led" }, { .id = CXT_FIXUP_HP_MIC_NO_PRESENCE, .name = "hp-mic-fix" }, + { .id = CXT_PINCFG_LENOVO_NOTEBOOK, .name = "lenovo-20149" }, {} }; -- 2.35.3

1 0

[PATCH] ALSA: ymfpci: remove unused snd_ymfpci_readb function
by Tom Rix 20 Mar '23

20 Mar '23

clang with W=1 reports sound/pci/ymfpci/ymfpci_main.c:34:18: error: unused function 'snd_ymfpci_readb' [-Werror,-Wunused-function] static inline u8 snd_ymfpci_readb(struct snd_ymfpci *chip, u32 offset) ^ This static function is not used, so remove it. Signed-off-by: Tom Rix <trix(a)redhat.com> --- sound/pci/ymfpci/ymfpci_main.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/sound/pci/ymfpci/ymfpci_main.c b/sound/pci/ymfpci/ymfpci_main.c index c80114c0ad7b..2858736ed20a 100644 --- a/sound/pci/ymfpci/ymfpci_main.c +++ b/sound/pci/ymfpci/ymfpci_main.c @@ -31,11 +31,6 @@ static void snd_ymfpci_irq_wait(struct snd_ymfpci *chip); -static inline u8 snd_ymfpci_readb(struct snd_ymfpci *chip, u32 offset) -{ - return readb(chip->reg_area_virt + offset); -} - static inline void snd_ymfpci_writeb(struct snd_ymfpci *chip, u32 offset, u8 val) { writeb(val, chip->reg_area_virt + offset); -- 2.27.0

3 3

Using mili/micro/nanoseconds in split files
by GitHub issues - opened 20 Mar '23

20 Mar '23

alsa-project/alsa-tools issue #15 was opened from kfilipan: This is more of a feature request rather than an issue. When using `arecord` with `--max-file-time` and `--use-strftime`, one cannot get nanoseconds in the file name. This is however available in the e.g. `date` command `date +"%Y-%m-%d_%H-%M-%S.%N"` (`%N` gives nanoseconds). This would be useful for testing sound card recording time delays because having only seconds is not precise enough, e.g. it could take days or weeks for even slightly better sound cards to experience a second of delay. Thank you! Issue URL : https://github.com/alsa-project/alsa-tools/issues/15 Repository URL: https://github.com/alsa-project/alsa-tools

1 0

[PATCH v1] ALSA:ac97: Remove redundant driver match function
by Lizhe 20 Mar '23

20 Mar '23

If there is no driver match function, the driver core assumes that each candidate pair (driver, device) matches, see driver_match_device() Drop the bus's match function that always returned 1 and so implements the same behaviour as when there is no match function. Signed-off-by: Lizhe <sensor1010(a)163.com> --- sound/ac97_bus.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/sound/ac97_bus.c b/sound/ac97_bus.c index b4685c53ff11..c7aee8c42c55 100644 --- a/sound/ac97_bus.c +++ b/sound/ac97_bus.c @@ -75,19 +75,8 @@ int snd_ac97_reset(struct snd_ac97 *ac97, bool try_warm, unsigned int id, } EXPORT_SYMBOL_GPL(snd_ac97_reset); -/* - * Let drivers decide whether they want to support given codec from their - * probe method. Drivers have direct access to the struct snd_ac97 - * structure and may decide based on the id field amongst other things. - */ -static int ac97_bus_match(struct device *dev, struct device_driver *drv) -{ - return 1; -} - struct bus_type ac97_bus_type = { .name = "ac97", - .match = ac97_bus_match, }; static int __init ac97_bus_init(void) -- 2.34.1

2 2

USB-Audio: ALC4080, add MSI MPG Z590 Gaming Force
by GitHub pull_request - opened 20 Mar '23

20 Mar '23

alsa-project/alsa-ucm-conf pull request #292 was opened from Paris17: USB ID: 0db0:8af7 Adds support for the MSI MPG Z590 GAMING FORCE motherboard front & back 3.5mm jacks. Request URL : https://github.com/alsa-project/alsa-ucm-conf/pull/292 Patch URL : https://github.com/alsa-project/alsa-ucm-conf/pull/292.patch Repository URL: https://github.com/alsa-project/alsa-ucm-conf

1 0

[PATCH] Add M3402RA to the list of quirks
by Enrico Belleri 20 Mar '23

20 Mar '23

Fix builtin microphone on ASUS Vivobook S 14 OLED 2022 (M3402RA) Same issue with this model as apparently with other Rembrandt laptops: https://bugzilla.kernel.org/show_bug.cgi?id=216270 Signed-off-by: Enrico Belleri <idesmi(a)protonmail.com> --- sound/soc/amd/yc/acp6x-mach.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sound/soc/amd/yc/acp6x-mach.c b/sound/soc/amd/yc/acp6x-mach.c index 4a69ce702..80dfb6036 100644 --- a/sound/soc/amd/yc/acp6x-mach.c +++ b/sound/soc/amd/yc/acp6x-mach.c @@ -213,6 +213,13 @@ static const struct dmi_system_id yc_acp_quirk_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "M5402RA"), } }, + { + .driver_data = &acp6x_card, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "ASUSTeK COMPUTER INC."), + DMI_MATCH(DMI_PRODUCT_NAME, "M3402RA"), + } + }, { .driver_data = &acp6x_card, .matches = { -- 2.40.0

1 0

Re: [PATCH] kernel/drivers: Remove redundant driver match function
by Uwe Kleine-König 19 Mar '23

19 Mar '23

Hello, On Thu, May 05, 2022 at 09:59:52PM -0700, lizhe wrote: > If there is no driver match function, the driver core assumes that each > candidate pair (driver, device) matches, see driver_match_device(). I wonder who is supposed to apply this patch. Either it should be split by file and go in via the respective maintainers, or it goes in via Greg's tree? I added Greg to To: for him to chime in. Best regards Uwe > Signed-off-by: lizhe <sensor1010(a)163.com> > --- > drivers/mfd/mcp-core.c | 6 ------ > drivers/nubus/bus.c | 6 ------ > drivers/s390/crypto/vfio_ap_drv.c | 6 ------ > drivers/scsi/scsi_debug.c | 7 ------- > drivers/target/loopback/tcm_loop.c | 7 ------- > drivers/w1/w1.c | 6 ------ > sound/ac97_bus.c | 11 ----------- > 7 files changed, 49 deletions(-) > > diff --git a/drivers/mfd/mcp-core.c b/drivers/mfd/mcp-core.c > index 2fa592c37c6f..281a9369f2b3 100644 > --- a/drivers/mfd/mcp-core.c > +++ b/drivers/mfd/mcp-core.c > @@ -20,11 +20,6 @@ > #define to_mcp(d) container_of(d, struct mcp, attached_device) > #define to_mcp_driver(d) container_of(d, struct mcp_driver, drv) > > -static int mcp_bus_match(struct device *dev, struct device_driver *drv) > -{ > - return 1; > -} > - > static int mcp_bus_probe(struct device *dev) > { > struct mcp *mcp = to_mcp(dev); > @@ -43,7 +38,6 @@ static void mcp_bus_remove(struct device *dev) > > static struct bus_type mcp_bus_type = { > .name = "mcp", > - .match = mcp_bus_match, > .probe = mcp_bus_probe, > .remove = mcp_bus_remove, > }; > diff --git a/drivers/nubus/bus.c b/drivers/nubus/bus.c > index 17fad660032c..72921e4f35f6 100644 > --- a/drivers/nubus/bus.c > +++ b/drivers/nubus/bus.c > @@ -14,11 +14,6 @@ > #define to_nubus_board(d) container_of(d, struct nubus_board, dev) > #define to_nubus_driver(d) container_of(d, struct nubus_driver, driver) > > -static int nubus_bus_match(struct device *dev, struct device_driver *driver) > -{ > - return 1; > -} > - > static int nubus_device_probe(struct device *dev) > { > struct nubus_driver *ndrv = to_nubus_driver(dev->driver); > @@ -39,7 +34,6 @@ static void nubus_device_remove(struct device *dev) > > struct bus_type nubus_bus_type = { > .name = "nubus", > - .match = nubus_bus_match, > .probe = nubus_device_probe, > .remove = nubus_device_remove, > }; > diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c > index 29ebd54f8919..0a662c451f2a 100644 > --- a/drivers/s390/crypto/vfio_ap_drv.c > +++ b/drivers/s390/crypto/vfio_ap_drv.c > @@ -172,14 +172,8 @@ static void vfio_ap_matrix_dev_release(struct device *dev) > kfree(matrix_dev); > } > > -static int matrix_bus_match(struct device *dev, struct device_driver *drv) > -{ > - return 1; > -} > - > static struct bus_type matrix_bus = { > .name = "matrix", > - .match = &matrix_bus_match, > }; > > static struct device_driver matrix_driver = { > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c > index 592a290e6cfa..8107489b36e8 100644 > --- a/drivers/scsi/scsi_debug.c > +++ b/drivers/scsi/scsi_debug.c > @@ -7844,15 +7844,8 @@ static void sdebug_driver_remove(struct device *dev) > scsi_host_put(sdbg_host->shost); > } > > -static int pseudo_lld_bus_match(struct device *dev, > - struct device_driver *dev_driver) > -{ > - return 1; > -} > - > static struct bus_type pseudo_lld_bus = { > .name = "pseudo", > - .match = pseudo_lld_bus_match, > .probe = sdebug_driver_probe, > .remove = sdebug_driver_remove, > .drv_groups = sdebug_drv_groups, > diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c > index 4407b56aa6d1..eeb63deff94f 100644 > --- a/drivers/target/loopback/tcm_loop.c > +++ b/drivers/target/loopback/tcm_loop.c > @@ -83,15 +83,8 @@ static int tcm_loop_show_info(struct seq_file *m, struct Scsi_Host *host) > static int tcm_loop_driver_probe(struct device *); > static void tcm_loop_driver_remove(struct device *); > > -static int pseudo_lld_bus_match(struct device *dev, > - struct device_driver *dev_driver) > -{ > - return 1; > -} > - > static struct bus_type tcm_loop_lld_bus = { > .name = "tcm_loop_bus", > - .match = pseudo_lld_bus_match, > .probe = tcm_loop_driver_probe, > .remove = tcm_loop_driver_remove, > }; > diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c > index f2ae2e563dc5..a6ecfa1b3417 100644 > --- a/drivers/w1/w1.c > +++ b/drivers/w1/w1.c > @@ -58,11 +58,6 @@ MODULE_PARM_DESC(slave_ttl, > DEFINE_MUTEX(w1_mlock); > LIST_HEAD(w1_masters); > > -static int w1_master_match(struct device *dev, struct device_driver *drv) > -{ > - return 1; > -} > - > static int w1_master_probe(struct device *dev) > { > return -ENODEV; > @@ -174,7 +169,6 @@ static int w1_uevent(struct device *dev, struct kobj_uevent_env *env); > > static struct bus_type w1_bus_type = { > .name = "w1", > - .match = w1_master_match, > .uevent = w1_uevent, > }; > > diff --git a/sound/ac97_bus.c b/sound/ac97_bus.c > index b4685c53ff11..c7aee8c42c55 100644 > --- a/sound/ac97_bus.c > +++ b/sound/ac97_bus.c > @@ -75,19 +75,8 @@ int snd_ac97_reset(struct snd_ac97 *ac97, bool try_warm, unsigned int id, > } > EXPORT_SYMBOL_GPL(snd_ac97_reset); > > -/* > - * Let drivers decide whether they want to support given codec from their > - * probe method. Drivers have direct access to the struct snd_ac97 > - * structure and may decide based on the id field amongst other things. > - */ > -static int ac97_bus_match(struct device *dev, struct device_driver *drv) > -{ > - return 1; > -} > - > struct bus_type ac97_bus_type = { > .name = "ac97", > - .match = ac97_bus_match, > }; > > static int __init ac97_bus_init(void) > -- > 2.25.1 > > -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |

3 2

Re:Re: [PATCH v5] serial: linflexuart: remove redundant uart type assignment
by lizhe 19 Mar '23

19 Mar '23

HI : My name is Li zhe. i am chinese At 2023-01-05 14:32:07, "Jiri Slaby" <jirislaby(a)kernel.org> wrote: >On 04. 01. 23, 17:43, LIZHE wrote: >> in linflex_config_port() the member variable type will be >> assigned again. see linflex_connfig_port() > >Please use capitals when you start a sentence. And periods at the ends >of them. > >> Signed-off-by: LIZHE <sensor1010(a)163.com> > >No, Lizhe was fine (or use your Chinese name. We support unicode so that >would be fine too, as Greg stated). > >But you didn't include changelog to your previous v1-v4 patches; as >stated by Ilpo. > >> Link: https://lore.kernel.org/all/20230103170948.3777-1-sensor1010@163.com/ >> --- >> drivers/tty/serial/fsl_linflexuart.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/drivers/tty/serial/fsl_linflexuart.c b/drivers/tty/serial/fsl_linflexuart.c >> index 6fc21b6684e6..34400cc6ac7f 100644 >> --- a/drivers/tty/serial/fsl_linflexuart.c >> +++ b/drivers/tty/serial/fsl_linflexuart.c >> @@ -837,7 +837,6 @@ static int linflex_probe(struct platform_device *pdev) >> return PTR_ERR(sport->membase); >> >> sport->dev = &pdev->dev; >> - sport->type = PORT_LINFLEXUART; >> sport->iotype = UPIO_MEM; >> sport->irq = platform_get_irq(pdev, 0); >> sport->ops = &linflex_pops; > >-- >js >suse labs

1 0

[PATCH] ALSA: drivers: Remove the unused variable portman_read_data
by Jiapeng Chong 19 Mar '23

19 Mar '23

Variable portman_read_data is not effectively used, so delete it. sound/drivers/portman2x4.c:195:18: warning: unused function 'portman_read_data'. Reported-by: Abaci Robot <abaci(a)linux.alibaba.com> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4557 Signed-off-by: Jiapeng Chong <jiapeng.chong(a)linux.alibaba.com> --- sound/drivers/portman2x4.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/sound/drivers/portman2x4.c b/sound/drivers/portman2x4.c index 52a656735365..41b46f9ae731 100644 --- a/sound/drivers/portman2x4.c +++ b/sound/drivers/portman2x4.c @@ -192,11 +192,6 @@ static inline u8 portman_read_status(struct portman *pm) return parport_read_status(pm->pardev->port); } -static inline u8 portman_read_data(struct portman *pm) -{ - return parport_read_data(pm->pardev->port); -} - static inline void portman_write_data(struct portman *pm, u8 value) { parport_write_data(pm->pardev->port, value); -- 2.20.1.7.g153144c

2 2