April 2021 - Alsa-devel - mailman.alsa-project.org

cplay: Add suport for AAC audio codec
by GitHub pull_request - opened 06 Apr '21

06 Apr '21

alsa-project/tinycompress pull request #10 was opened from TangleZ: Add parse_aac_header API to parse AAC streams, support ADTS and ADIF format. Signed-off-by: Zhang Peng <peng.zhang_8(a)nxp.com> Request URL : https://github.com/alsa-project/tinycompress/pull/10 Patch URL : https://github.com/alsa-project/tinycompress/pull/10.patch Repository URL: https://github.com/alsa-project/tinycompress

1 0

[PATCH] ALSA: usb-audio: Drop implicit fb quirk entries dubbed for capture
by Takashi Iwai 06 Apr '21

06 Apr '21

The implicit feedback quirk table contains the entries that also appear in the capture quirks, and those are all handled to be skipped. For the code simplicity, drop the duped entries in the playback quirk table, and check the match with the capture quirk table instead. BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=212519 Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/usb/implicit.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/sound/usb/implicit.c b/sound/usb/implicit.c index 11a85e66aa96..19622c58b72b 100644 --- a/sound/usb/implicit.c +++ b/sound/usb/implicit.c @@ -70,16 +70,6 @@ static const struct snd_usb_implicit_fb_match playback_implicit_fb_quirks[] = { .type = IMPLICIT_FB_FIXED, .ep_num = 0x84, .iface = 0 }, /* MOTU MicroBook II */ - /* No quirk for playback but with capture quirk (see below) */ - IMPLICIT_FB_SKIP_DEV(0x0582, 0x0130), /* BOSS BR-80 */ - IMPLICIT_FB_SKIP_DEV(0x0582, 0x0171), /* BOSS RC-505 */ - IMPLICIT_FB_SKIP_DEV(0x0582, 0x0185), /* BOSS GP-10 */ - IMPLICIT_FB_SKIP_DEV(0x0582, 0x0189), /* BOSS GT-100v2 */ - IMPLICIT_FB_SKIP_DEV(0x0582, 0x01d6), /* BOSS GT-1 */ - IMPLICIT_FB_SKIP_DEV(0x0582, 0x01d8), /* BOSS Katana */ - IMPLICIT_FB_SKIP_DEV(0x0582, 0x01e5), /* BOSS GT-001 */ - IMPLICIT_FB_SKIP_DEV(0x0582, 0x0203), /* BOSS AD-10 */ - {} /* terminator */ }; @@ -278,6 +268,11 @@ static int audioformat_implicit_fb_quirk(struct snd_usb_audio *chip, } } + /* Don't apply playback quirks for the devices with capture quirk */ + p = find_implicit_fb_entry(chip, capture_implicit_fb_quirks, alts); + if (p && p->type == IMPLICIT_FB_FIXED) + return 0; /* no quirk */ + /* Generic UAC2 implicit feedback */ if (attr == USB_ENDPOINT_SYNC_ASYNC && alts->desc.bInterfaceClass == USB_CLASS_AUDIO && -- 2.26.2

1 0

[PATCH] ALSA: usb-audio: Add error checks for usb_driver_claim_interface() calls
by Takashi Iwai 06 Apr '21

06 Apr '21

There are a few calls of usb_driver_claim_interface() but all of those miss the proper error checks, as reported by Coverity. This patch adds those missing checks. Along with it, replace the magic pointer with -1 with a constant USB_AUDIO_IFACE_UNUSED for better readability. Reported-by: coverity-bot <keescook+coverity-bot(a)chromium.org> Addresses-Coverity-ID: 1475943 ("Error handling issues") Addresses-Coverity-ID: 1475944 ("Error handling issues") Addresses-Coverity-ID: 1475945 ("Error handling issues") Fixes: b1ce7ba619d9 ("ALSA: usb-audio: claim autodetected PCM interfaces all at once") Fixes: e5779998bf8b ("ALSA: usb-audio: refactor code") Link: https://lore.kernel.org/r/202104051059.FB7F3016@keescook Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/usb/card.c | 14 +++++++------- sound/usb/quirks.c | 16 ++++++++++++---- sound/usb/usbaudio.h | 2 ++ 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/sound/usb/card.c b/sound/usb/card.c index 0826a437f8fc..7b7526d3a56e 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -181,9 +181,8 @@ static int snd_usb_create_stream(struct snd_usb_audio *chip, int ctrlif, int int ctrlif, interface); return -EINVAL; } - usb_driver_claim_interface(&usb_audio_driver, iface, (void *)-1L); - - return 0; + return usb_driver_claim_interface(&usb_audio_driver, iface, + USB_AUDIO_IFACE_UNUSED); } if ((altsd->bInterfaceClass != USB_CLASS_AUDIO && @@ -203,7 +202,8 @@ static int snd_usb_create_stream(struct snd_usb_audio *chip, int ctrlif, int int if (! snd_usb_parse_audio_interface(chip, interface)) { usb_set_interface(dev, interface, 0); /* reset the current interface */ - usb_driver_claim_interface(&usb_audio_driver, iface, (void *)-1L); + return usb_driver_claim_interface(&usb_audio_driver, iface, + USB_AUDIO_IFACE_UNUSED); } return 0; @@ -862,7 +862,7 @@ static void usb_audio_disconnect(struct usb_interface *intf) struct snd_card *card; struct list_head *p; - if (chip == (void *)-1L) + if (chip == USB_AUDIO_IFACE_UNUSED) return; card = chip->card; @@ -992,7 +992,7 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message) struct usb_mixer_interface *mixer; struct list_head *p; - if (chip == (void *)-1L) + if (chip == USB_AUDIO_IFACE_UNUSED) return 0; if (!chip->num_suspended_intf++) { @@ -1022,7 +1022,7 @@ static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume) struct list_head *p; int err = 0; - if (chip == (void *)-1L) + if (chip == USB_AUDIO_IFACE_UNUSED) return 0; atomic_inc(&chip->active); /* avoid autopm */ diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index ae667cf77e61..63f27ce64bcb 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -55,8 +55,12 @@ static int create_composite_quirk(struct snd_usb_audio *chip, if (!iface) continue; if (quirk->ifnum != probed_ifnum && - !usb_interface_claimed(iface)) - usb_driver_claim_interface(driver, iface, (void *)-1L); + !usb_interface_claimed(iface)) { + err = usb_driver_claim_interface(driver, iface, + USB_AUDIO_IFACE_UNUSED); + if (err < 0) + return err; + } } return 0; @@ -426,8 +430,12 @@ static int create_autodetect_quirks(struct snd_usb_audio *chip, continue; err = create_autodetect_quirk(chip, iface, driver); - if (err >= 0) - usb_driver_claim_interface(driver, iface, (void *)-1L); + if (err >= 0) { + err = usb_driver_claim_interface(driver, iface, + USB_AUDIO_IFACE_UNUSED); + if (err < 0) + return err; + } } return 0; diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index 60b9dd7df6bb..8794c8658ab9 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -61,6 +61,8 @@ struct snd_usb_audio { struct media_intf_devnode *ctl_intf_media_devnode; }; +#define USB_AUDIO_IFACE_UNUSED ((void *)-1L) + #define usb_audio_err(chip, fmt, args...) \ dev_err(&(chip)->dev->dev, fmt, ##args) #define usb_audio_warn(chip, fmt, args...) \ -- 2.26.2

1 0

snd_pcm_htimestamp returns time past since last boot not clock time
by GitHub issues - opened 06 Apr '21

06 Apr '21

alsa-project/alsa-lib issue #131 was opened from RonaldAJ: While on a PC snd_pcm_htimestamp returns the clock time since the epoch, under Raspbian on an RPI it returns the time since the last boot. (A similar issue was described here: https://stackoverflow.com/questions/45841230/alsa-retrieving-audio-buffer-t…). The problem shows if we run a slightly modified version of recordtest.py using the latest pyalsaaudio versions. ``` #!/usr/bin/env python3 # -*- mode: python; indent-tabs-mode: t; c-basic-offset: 4; tab-width: 4 -*- ## recordtest.py ## ## This is an example of a simple sound capture script. ## ## The script opens an ALSA pcm device for sound capture, sets ## various attributes of the capture, and reads in a loop, ## writing the data to standard out. ## ## To test it out do the following: ## python recordtest.py out.raw # talk to the microphone ## aplay -r 8000 -f S16_LE -c 1 out.raw #!/usr/bin/env python from __future__ import print_function import sys import time import getopt import alsaaudio def usage(): print('usage: recordtest.py [-d <device>] <file>', file=sys.stderr) sys.exit(2) if __name__ == '__main__': device = 'default' opts, args = getopt.getopt(sys.argv[1:], 'd:') for o, a in opts: if o == '-d': device = a if not args: usage() f = open(args[0], 'wb') # Open the device in nonblocking capture mode in mono, with a sampling rate of 44100 Hz # and 16 bit little endian samples # The period size controls the internal number of frames per period. # The significance of this parameter is documented in the ALSA api. # For our purposes, it is suficcient to know that reads from the device # will return this many frames. Each frame being 2 bytes long. # This means that the reads below will return either 320 bytes of data # or 0 bytes of data. The latter is possible because we are in nonblocking # mode. inp = alsaaudio.PCM(alsaaudio.PCM_CAPTURE, alsaaudio.PCM_NONBLOCK, channels=1, rate=44100, format=alsaaudio.PCM_FORMAT_S16_LE, periodsize=160, device=device) print(inp.info()) # help(inp.htimestamp) loops = 100000 while loops > 0: loops -= 1 # Read data from device l, data = inp.read() if l: print(l, inp.htimestamp(), time.time()) f.write(data) time.sleep(.001) ``` When executed on an RPi it yields: ``` {'name': 'plugsnoop:1,0', 'state': 'PREPARED', 'access_type': 'RW_INTERLEAVED', ' (call value) type': 1, ' (call value) type_name': 'CAPTURE', ' (call value) mode': 1, ' (call value) mode_name': 'PCM_NONBLOCK', 'format': 2, 'format_name': 'S16_LE', 'format_description': 'Signed 16 bit Little Endian', 'subformat_name': 'STD', 'subformat_description': 'Standard', 'channels': 1, 'rate': 44100, 'period_time': 21333, 'period_size': 940, 'buffer_time': 0, 'buffer_size': 4704, 'get_periods': 0, 'rate_numden': (44100, 1), 'significant_bits': 16, 'is_batch': True, 'is_block_transfer': True, 'is_double': False, 'is_half_duplex': False, 'is_joint_duplex': False, 'can_overrange': False, 'can_mmap_sample_resolution': False, 'can_pause': False, 'can_resume': False, 'can_sync_start': False} 940 (5596, 851796145, 0) 1617702646.0637105 940 (5596, 872796030, 0) 1617702646.084714 940 (5596, 893795916, 0) 1617702646.105702 940 (5596, 915796158, 0) 1617702646.1277127 940 (5596, 936795314, 0) 1617702646.1486921 940 (5596, 957795199, 0) 1617702646.1697078 940 (5596, 979795858, 0) 1617702646.191694 940 (5597, 795171, 0) 1617702646.212712 ... ``` I read that for the timestamp function to work there should be hardware support. But here we have it half working. This behavior is shown if I use the following in my .asoundrc ``` pcm.plugsnoop { type plug @args [ CARD DEV ] @args.CARD { type string } @args.DEV { type integer } slave { pcm { @func concat strings [ "dsnoop:" $CARD "," $DEV ] } } } ``` and call it with: `python recordtest.py -d plugsnoop:1,0 test.raw` Using plughw all timestamps become zero. The attached microphone is an USB microphone: GoMic by SAMSON. Is there a fix or workaround for this behavior which gives me clocktime instead of seconds since last boot? Issue URL : https://github.com/alsa-project/alsa-lib/issues/131 Repository URL: https://github.com/alsa-project/alsa-lib

1 0

RE: [PATCH v6 2/2] ASoC: rt715:add micmute led state control supportspobrn@protonmail.com
by Yuan, Perry 06 Apr '21

06 Apr '21

Hi Jaroslav: > -----Original Message----- > From: Yuan, Perry <Perry_Yuan(a)Dell.com> > Sent: Sunday, April 4, 2021 4:32 PM > To: pobrn(a)protonmail.com; pierre-louis.bossart(a)linux.intel.com; > oder_chiou(a)realtek.com; perex(a)perex.cz; tiwai(a)suse.com; > hdegoede(a)redhat.com; mgross(a)linux.intel.com; Limonciello, Mario > Cc: lgirdwood(a)gmail.com; broonie(a)kernel.org; alsa-devel(a)alsa-project.org; > linux-kernel(a)vger.kernel.org; platform-driver-x86(a)vger.kernel.org; Yuan, > Perry; mario.limonciello(a)outlook.com; Dell Client Kernel > Subject: [PATCH v6 2/2] ASoC: rt715:add micmute led state control supports > > From: Perry Yuan <perry_yuan(a)dell.com> > > Some new Dell system is going to support audio internal micphone privacy > setting from hardware level with micmute led state changing When micmute > hotkey pressed by user, soft mute will need to be enabled firstly in case of > pop noise, and codec driver need to react to mic mute event to > EC(embedded controller) notifying that SW mute is completed Then EC will > do the hardware mute physically within the timeout reached > > This patch allow codec rt715 and rt715 sdca driver to change the local > micmute led state. Dell privacy led trigger driver will ack EC when micmute > key pressed through this micphone led control interface like hda_generic > provided ACPI method defined in dell-privacy micmute led trigger will be > called for notifying the EC that software mute has been completed, then > hardware audio circuit solution controlled by EC will switch the audio input > source off/on > > Signed-off-by: Perry Yuan <perry_yuan(a)dell.com> > > -------- > v5 -> v6: > * addresed review comments from Jaroslav > * add quirks for micmute led control as short term solution to control > micmute led state change > v4 -> v5: > * rebase to latest 5.12 rc4 upstream kernel > v3 -> v4: > * remove unused debug log > * remove compile flag of DELL privacy > * move the micmute_led to local from rt715_priv > * when Jaroslav upstream his gerneric LED trigger driver,I will rebase > this patch,please consider merge this at first > https://lore.kernel.org/alsa-devel/20210211111400.1131020-1- > perex(a)perex.cz/ > v2 -> v3: > * simplify the patch to reuse some val value > * add more detail to the commit info > v1 -> v2: > * fix some format issue > -------- > --- > sound/soc/codecs/rt715-sdca.c | 41 > ++++++++++++++++++++++++++++++++++- > sound/soc/codecs/rt715.c | 41 > +++++++++++++++++++++++++++++++++++ > 2 files changed, 81 insertions(+), 1 deletion(-) > > diff --git a/sound/soc/codecs/rt715-sdca.c b/sound/soc/codecs/rt715-sdca.c > index 20528afbdc57..7bd7ad0ba7d7 100644 > --- a/sound/soc/codecs/rt715-sdca.c > +++ b/sound/soc/codecs/rt715-sdca.c > @@ -11,12 +11,14 @@ > #include <linux/moduleparam.h> > #include <linux/kernel.h> > #include <linux/init.h> > +#include <linux/leds.h> > #include <linux/pm_runtime.h> > #include <linux/pm.h> > #include <linux/soundwire/sdw.h> > #include <linux/regmap.h> > #include <linux/slab.h> > #include <linux/platform_device.h> > +#include <linux/dmi.h> > #include <sound/core.h> > #include <sound/pcm.h> > #include <sound/pcm_params.h> > @@ -344,6 +346,34 @@ static int rt715_sdca_get_volsw(struct snd_kcontrol > *kcontrol, > return 0; > } > > +static bool micmute_led_set; > +static int dmi_matched(const struct dmi_system_id *dmi) { > + micmute_led_set = 1; > + return 1; > +} > + > +/* Some systems will need to use this to trigger mic mute LED state > +changed */ static const struct dmi_system_id micmute_led_dmi_table[] = { > + { > + .callback = dmi_matched, > + .ident = "Dell Latitude 9420", > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), > + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude > 9420"), > + }, > + }, > + { > + .callback = dmi_matched, > + .ident = "Dell Latitude 9520", > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), > + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude > 9520"), > + }, > + }, > + {}, > +}; > + > static int rt715_sdca_put_volsw(struct snd_kcontrol *kcontrol, > struct snd_ctl_elem_value *ucontrol) > { > @@ -358,6 +388,7 @@ static int rt715_sdca_put_volsw(struct snd_kcontrol > *kcontrol, > unsigned int mask = (1 << fls(max)) - 1; > unsigned int invert = p->invert; > int err; > + bool micmute_led; > > for (i = 0; i < 4; i++) { > if (ucontrol->value.integer.value[i] != rt715- > >kctl_switch_orig[i]) { @@ -393,7 +424,15 @@ static int > rt715_sdca_put_volsw(struct snd_kcontrol *kcontrol, > if (err < 0) > return err; > } > - > + /* Micmute LED state changed by muted/unmute switch */ > + dmi_check_system(micmute_led_dmi_table); > + if (invert && micmute_led_set) { > + if (ucontrol->value.integer.value[0] || ucontrol- > >value.integer.value[1]) > + micmute_led = LED_OFF; > + else > + micmute_led = LED_ON; > + ledtrig_audio_set(LED_AUDIO_MICMUTE, micmute_led); > + } > return k_changed; > } > > diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c index > 34c3357e943b..490cf9e63c2d 100644 > --- a/sound/soc/codecs/rt715.c > +++ b/sound/soc/codecs/rt715.c > @@ -13,6 +13,7 @@ > #include <linux/init.h> > #include <linux/delay.h> > #include <linux/i2c.h> > +#include <linux/leds.h> > #include <linux/pm_runtime.h> > #include <linux/pm.h> > #include <linux/soundwire/sdw.h> > @@ -25,6 +26,7 @@ > #include <linux/of.h> > #include <linux/of_gpio.h> > #include <linux/of_device.h> > +#include <linux/dmi.h> > #include <sound/core.h> > #include <sound/pcm.h> > #include <sound/pcm_params.h> > @@ -70,6 +72,34 @@ static void rt715_get_gain(struct rt715_priv *rt715, > unsigned int addr_h, > pr_err("Failed to get L channel gain.\n"); } > > +static bool micmute_led_set; > +static int dmi_matched(const struct dmi_system_id *dmi) { > + micmute_led_set = 1; > + return 1; > +} > + > +/* Some systems will need to use this to trigger mic mute LED state > +changed */ static const struct dmi_system_id micmute_led_dmi_table[] = { > + { > + .callback = dmi_matched, > + .ident = "Dell Latitude 9420", > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), > + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude > 9420"), > + }, > + }, > + { > + .callback = dmi_matched, > + .ident = "Dell Latitude 9520", > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), > + DMI_MATCH(DMI_PRODUCT_NAME, "Latitude > 9520"), > + }, > + }, > + {}, > +}; > + > /* For Verb-Set Amplifier Gain (Verb ID = 3h) */ static int > rt715_set_amp_gain_put(struct snd_kcontrol *kcontrol, > struct snd_ctl_elem_value *ucontrol) > @@ -88,6 +118,7 @@ static int rt715_set_amp_gain_put(struct snd_kcontrol > *kcontrol, > RT715_SET_GAIN_MIX_ADC2_L}; > unsigned int addr_h, addr_l, val_h, val_ll, val_lr; > unsigned int read_ll, read_rl, i, j, loop_cnt; > + bool micmute_led; > > if (strstr(ucontrol->id.name, "Main Capture Switch") || > strstr(ucontrol->id.name, "Main Capture Volume")) @@ - > 185,6 +216,16 @@ static int rt715_set_amp_gain_put(struct snd_kcontrol > *kcontrol, > if (dapm->bias_level <= SND_SOC_BIAS_STANDBY) > regmap_write(rt715->regmap, > RT715_SET_AUDIO_POWER_STATE, > AC_PWRST_D3); > + > + /* Micmute LED state changed by muted/unmute switch */ > + dmi_check_system(micmute_led_dmi_table); > + if (mc->invert && micmute_led_set) { > + if (ucontrol->value.integer.value[0] || ucontrol- > >value.integer.value[1]) > + micmute_led = LED_OFF; > + else > + micmute_led = LED_ON; > + ledtrig_audio_set(LED_AUDIO_MICMUTE, micmute_led); > + } > return 0; > } > > -- > 2.25.1 Could you help to review if the patch is acceptable as short term solution? I added dmi checking when changing the led state for rt715 and rt715-sdca codec. Thank you! Perry

1 0

[ANNOUNCE] SOF v1.7 released
by Girdwood, Liam R 06 Apr '21

06 Apr '21

The SOF developers are pleased to announce that version 1.7 has been released and binaries available for Intel and NXP platforms[1]. The main focus of v1.7 was around adding support for new audio processing algorithms, continued Zephyr integration and general stabilisation. Release notes are here. https://github.com/thesofproject/sof/releases/tag/v1.7 Binaries are here. https://github.com/thesofproject/sof-bin Firmware issues can be reported here (and feature requests too). https://github.com/thesofproject/sof/issues and kernel issues here https://github.com/thesofproject/linux/issues The next version will be v1.8 due by the end of June. Thanks Liam [1] Preliminary support for other platforms can be enabled by using SOF as a Zephyr module (for Zephyr supported architectures and platforms). --------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

2 1

[PATCH v2] soundwire: intel_init: test link->cdns
by Bard Liao 06 Apr '21

06 Apr '21

intel_link_probe() could return error and dev_get_drvdata() will return null in such case. So we have to test link->cdns after link->cdns = dev_get_drvdata(&ldev->auxdev.dev); Otherwise, we will meet the "kernel NULL pointer dereference" error. Signed-off-by: Bard Liao <yung-chuan.liao(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Reviewed-by: Rander Wang <rander.wang(a)intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> --- v2: - Rebase to latest code base. --- drivers/soundwire/intel_init.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c index 05b726cdfebc..30ce95ec2d70 100644 --- a/drivers/soundwire/intel_init.c +++ b/drivers/soundwire/intel_init.c @@ -178,6 +178,15 @@ static struct sdw_intel_ctx link->pdev = pdev; link->cdns = platform_get_drvdata(pdev); + if (!link->cdns) { + dev_err(&adev->dev, "failed to get link->cdns\n"); + /* + * 1 will be subtracted from i in the err label, but we need to call + * intel_link_dev_unregister for this ldev, so plus 1 now + */ + i++; + goto err; + } list_add_tail(&link->list, &ctx->link_list); bus = &link->cdns->bus; /* Calculate number of slaves */ -- 2.17.1

2 1

[PATCH] soundwire: qcom: handle return correctly in qcom_swrm_transport_params
by Srinivas Kandagatla 06 Apr '21

06 Apr '21

Looks like return from reg_write is set but not checked. Fix this by adding error return path. Reported-by: coverity-bot <keescook+coverity-bot(a)chromium.org> Addresses-Coverity-ID: 1503591 ("UNUSED_VALUE") Fixes: 128eaf937adb ("soundwire: qcom: add support to missing transport params") Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla(a)linaro.org> --- drivers/soundwire/qcom.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 5fd4a99cc8ac..348d9a46f850 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -731,17 +731,23 @@ static int qcom_swrm_transport_params(struct sdw_bus *bus, value |= pcfg->si; ret = ctrl->reg_write(ctrl, reg, value); + if (ret) + goto err; if (pcfg->lane_control != SWR_INVALID_PARAM) { reg = SWRM_DP_PORT_CTRL_2_BANK(params->port_num, bank); value = pcfg->lane_control; ret = ctrl->reg_write(ctrl, reg, value); + if (ret) + goto err; } if (pcfg->blk_group_count != SWR_INVALID_PARAM) { reg = SWRM_DP_BLOCK_CTRL2_BANK(params->port_num, bank); value = pcfg->blk_group_count; ret = ctrl->reg_write(ctrl, reg, value); + if (ret) + goto err; } if (pcfg->hstart != SWR_INVALID_PARAM @@ -755,11 +761,15 @@ static int qcom_swrm_transport_params(struct sdw_bus *bus, ret = ctrl->reg_write(ctrl, reg, value); } + if (ret) + goto err; + if (pcfg->bp_mode != SWR_INVALID_PARAM) { reg = SWRM_DP_BLOCK_CTRL3_BANK(params->port_num, bank); ret = ctrl->reg_write(ctrl, reg, pcfg->bp_mode); } +err: return ret; } -- 2.21.0

3 2

[PATCH v2] soundwire: qcom: wait for fifo space to be available before read/write
by Srinivas Kandagatla 06 Apr '21

06 Apr '21

If we write registers very fast we can endup in a situation where some of the writes will be dropped without any notice. So wait for the fifo space to be available before reading/writing the soundwire registers. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla(a)linaro.org> --- Changes since v1: merged some of the loop code to make it simple as suggested by Pierre updated error code and comments as suggested by Vinod drivers/soundwire/qcom.c | 66 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 6affa3cd4039..5fd4a99cc8ac 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -24,6 +24,8 @@ #define SWRM_COMP_CFG_IRQ_LEVEL_OR_PULSE_MSK BIT(1) #define SWRM_COMP_CFG_ENABLE_MSK BIT(0) #define SWRM_COMP_PARAMS 0x100 +#define SWRM_COMP_PARAMS_WR_FIFO_DEPTH GENMASK(14, 10) +#define SWRM_COMP_PARAMS_RD_FIFO_DEPTH GENMASK(19, 15) #define SWRM_COMP_PARAMS_DOUT_PORTS_MASK GENMASK(4, 0) #define SWRM_COMP_PARAMS_DIN_PORTS_MASK GENMASK(9, 5) #define SWRM_INTERRUPT_STATUS 0x200 @@ -51,6 +53,8 @@ #define SWRM_CMD_FIFO_CMD 0x308 #define SWRM_CMD_FIFO_FLUSH 0x1 #define SWRM_CMD_FIFO_STATUS 0x30C +#define SWRM_RD_CMD_FIFO_CNT_MASK GENMASK(20, 16) +#define SWRM_WR_CMD_FIFO_CNT_MASK GENMASK(12, 8) #define SWRM_CMD_FIFO_CFG_ADDR 0x314 #define SWRM_CONTINUE_EXEC_ON_CMD_IGNORE BIT(31) #define SWRM_RD_WR_CMD_RETRIES 0x7 @@ -104,6 +108,7 @@ #define SWR_BROADCAST_CMD_ID 0x0F #define SWR_MAX_CMD_ID 14 #define MAX_FIFO_RD_RETRY 3 +#define SWR_OVERFLOW_RETRY_COUNT 30 struct qcom_swrm_port_config { u8 si; @@ -147,6 +152,8 @@ struct qcom_swrm_ctrl { int (*reg_read)(struct qcom_swrm_ctrl *ctrl, int reg, u32 *val); int (*reg_write)(struct qcom_swrm_ctrl *ctrl, int reg, int val); u32 slave_status; + u32 wr_fifo_depth; + u32 rd_fifo_depth; }; struct qcom_swrm_data { @@ -238,6 +245,55 @@ static u32 swrm_get_packed_reg_val(u8 *cmd_id, u8 cmd_data, return val; } +static int swrm_wait_for_rd_fifo_avail(struct qcom_swrm_ctrl *swrm) +{ + u32 fifo_outstanding_data, value; + int fifo_retry_count = SWR_OVERFLOW_RETRY_COUNT; + + do { + /* Check for fifo underflow during read */ + swrm->reg_read(swrm, SWRM_CMD_FIFO_STATUS, &value); + fifo_outstanding_data = FIELD_GET(SWRM_RD_CMD_FIFO_CNT_MASK, value); + + /* Check if read data is available in read fifo */ + if (fifo_outstanding_data > 0) + return 0; + + usleep_range(500, 510); + } while (fifo_retry_count--); + + if (fifo_outstanding_data == 0) { + dev_err_ratelimited(swrm->dev, "%s err read underflow\n", __func__); + return -EIO; + } + + return 0; +} + +static int swrm_wait_for_wr_fifo_avail(struct qcom_swrm_ctrl *swrm) +{ + u32 fifo_outstanding_cmds, value; + int fifo_retry_count = SWR_OVERFLOW_RETRY_COUNT; + + do { + /* Check for fifo overflow during write */ + swrm->reg_read(swrm, SWRM_CMD_FIFO_STATUS, &value); + fifo_outstanding_cmds = FIELD_GET(SWRM_WR_CMD_FIFO_CNT_MASK, value); + + /* Check for space in write fifo before writing */ + if (fifo_outstanding_cmds < swrm->wr_fifo_depth) + return 0; + + usleep_range(500, 510); + } while (fifo_retry_count--); + + if (fifo_outstanding_cmds == swrm->wr_fifo_depth) { + dev_err_ratelimited(swrm->dev, "%s err write overflow\n", __func__); + return -EIO; + } + + return 0; +} static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *swrm, u8 cmd_data, u8 dev_addr, u16 reg_addr) @@ -256,6 +312,9 @@ static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *swrm, u8 cmd_data, dev_addr, reg_addr); } + if (swrm_wait_for_wr_fifo_avail(swrm)) + return SDW_CMD_FAIL_OTHER; + /* Its assumed that write is okay as we do not get any status back */ swrm->reg_write(swrm, SWRM_CMD_FIFO_WR_CMD, val); @@ -295,6 +354,9 @@ static int qcom_swrm_cmd_fifo_rd_cmd(struct qcom_swrm_ctrl *swrm, /* wait for FIFO RD CMD complete to avoid overflow */ usleep_range(250, 255); + if (swrm_wait_for_rd_fifo_avail(swrm)) + return SDW_CMD_FAIL_OTHER; + do { swrm->reg_read(swrm, SWRM_CMD_FIFO_RD_FIFO_ADDR, &cmd_data); rval[0] = cmd_data & 0xFF; @@ -586,6 +648,10 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl) SWRM_INTERRUPT_STATUS_RMSK); } ctrl->slave_status = 0; + ctrl->reg_read(ctrl, SWRM_COMP_PARAMS, &val); + ctrl->rd_fifo_depth = FIELD_GET(SWRM_COMP_PARAMS_RD_FIFO_DEPTH, val); + ctrl->wr_fifo_depth = FIELD_GET(SWRM_COMP_PARAMS_WR_FIFO_DEPTH, val); + return 0; } -- 2.21.0

3 3

[PATCH] soundwire: qcom: cleanup internal port config indexing
by Srinivas Kandagatla 06 Apr '21

06 Apr '21

Internally used portconfig array for storing port bandwidth params starts from offset zero. However port zero is not really used and we also copy the bus parameters to offset zero. So basically we endup with a code which has to subtract 1 from port number to get to port parameters. This is bit confusing to the reader so, make this bit more obvious by only copying the parameters to offset 1 instead of zero. This will avoid doing -1 every time when we try to get port params. Similar thing has been recently done with din/dout_port_mask. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla(a)linaro.org> --- drivers/soundwire/qcom.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 94fd58f9dda3..348d9a46f850 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -724,7 +724,7 @@ static int qcom_swrm_transport_params(struct sdw_bus *bus, int reg = SWRM_DP_PORT_CTRL_BANK((params->port_num), bank); int ret; - pcfg = &ctrl->pconfig[params->port_num - 1]; + pcfg = &ctrl->pconfig[params->port_num]; value = pcfg->off1 << SWRM_DP_PORT_CTRL_OFFSET1_SHFT; value |= pcfg->off2 << SWRM_DP_PORT_CTRL_OFFSET2_SHFT; @@ -811,11 +811,11 @@ static int qcom_swrm_compute_params(struct sdw_bus *bus) struct qcom_swrm_port_config *pcfg; struct sdw_slave *slave; unsigned int m_port; - int i = 0; + int i = 1; list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) { list_for_each_entry(p_rt, &m_rt->port_list, port_node) { - pcfg = &ctrl->pconfig[p_rt->num - 1]; + pcfg = &ctrl->pconfig[p_rt->num]; p_rt->transport_params.port_num = p_rt->num; if (pcfg->word_length != SWR_INVALID_PARAM) { sdw_fill_port_params(&p_rt->port_params, @@ -832,7 +832,7 @@ static int qcom_swrm_compute_params(struct sdw_bus *bus) m_port = slave->m_port_map[p_rt->num]; /* port config starts at offset 0 so -1 from actual port number */ if (m_port) - pcfg = &ctrl->pconfig[m_port - 1]; + pcfg = &ctrl->pconfig[m_port]; else pcfg = &ctrl->pconfig[i]; p_rt->transport_params.port_num = p_rt->num; @@ -1167,15 +1167,16 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl) of_property_read_u8_array(np, "qcom,ports-lane-control", lane_control, nports); for (i = 0; i < nports; i++) { - ctrl->pconfig[i].si = si[i]; - ctrl->pconfig[i].off1 = off1[i]; - ctrl->pconfig[i].off2 = off2[i]; - ctrl->pconfig[i].bp_mode = bp_mode[i]; - ctrl->pconfig[i].hstart = hstart[i]; - ctrl->pconfig[i].hstop = hstop[i]; - ctrl->pconfig[i].word_length = word_length[i]; - ctrl->pconfig[i].blk_group_count = blk_group_count[i]; - ctrl->pconfig[i].lane_control = lane_control[i]; + /* Valid port number range is from 1-14 */ + ctrl->pconfig[i + 1].si = si[i]; + ctrl->pconfig[i + 1].off1 = off1[i]; + ctrl->pconfig[i + 1].off2 = off2[i]; + ctrl->pconfig[i + 1].bp_mode = bp_mode[i]; + ctrl->pconfig[i + 1].hstart = hstart[i]; + ctrl->pconfig[i + 1].hstop = hstop[i]; + ctrl->pconfig[i + 1].word_length = word_length[i]; + ctrl->pconfig[i + 1].blk_group_count = blk_group_count[i]; + ctrl->pconfig[i + 1].lane_control = lane_control[i]; } return 0; -- 2.21.0

2 1