September 2020 - Alsa-devel - mailman.alsa-project.org

[PATCH] ALSA: rockchip: fix a possible divide-by-zero bug in rockchip_i2s_hw_params()
by Tuo Li 25 Sep '20

25 Sep '20

The variable bclk_rate is checked in: if (bclk_rate && mclk_rate % bclk_rate) This indicates that bclk_rate can be zero. If so, a divide-by-zero bug will occur: div_bclk = mclk_rate / bclk_rate; To fix this possible bug, the function returns -EINVAL when bclk_rate is zero. Signed-off-by: Tuo Li <t-li20(a)mails.tsinghua.edu.cn> --- sound/soc/rockchip/rockchip_i2s.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index d1438753edb4..dd0836c32639 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -279,7 +279,9 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream, if (i2s->is_master_mode) { mclk_rate = clk_get_rate(i2s->mclk); bclk_rate = 2 * 32 * params_rate(params); - if (bclk_rate && mclk_rate % bclk_rate) + if (bclk_rate == 0) + return -EINVAL; + if (mclk_rate % bclk_rate) return -EINVAL; div_bclk = mclk_rate / bclk_rate; -- 2.17.1

2 2

Re: [greybus-dev] [PATCH 3/3] [PATCH] staging: greybus: __u8 is sufficient for snd_ctl_elem_type_t and snd_ctl_elem_iface_t
by Alex Elder 25 Sep '20

25 Sep '20

On 9/25/20 9:11 AM, Coiby Xu wrote: > On Thu, Sep 24, 2020 at 10:54:50AM +0000, David Laight wrote: >> From: Coiby Xu >>> Sent: 24 September 2020 11:21 >>> Use __8 to replace int and remove the unnecessary __bitwise type >>> attribute. >>> >>> Found by sparse, >> ... >>> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h >>> index 535a7229e1d9..8e71a95644ab 100644 >>> --- a/include/uapi/sound/asound.h >>> +++ b/include/uapi/sound/asound.h >>> @@ -950,7 +950,7 @@ struct snd_ctl_card_info { >>> unsigned char components[128]; /* card components / fine >>> identification, delimited with one >>> space (AC97 etc..) */ >>> }; >>> >>> -typedef int __bitwise snd_ctl_elem_type_t; >>> +typedef __u8 snd_ctl_elem_type_t; >>> #define SNDRV_CTL_ELEM_TYPE_NONE ((__force >>> snd_ctl_elem_type_t) 0) /* invalid */ >>> #define SNDRV_CTL_ELEM_TYPE_BOOLEAN ((__force >>> snd_ctl_elem_type_t) 1) /* boolean type */ >>> #define SNDRV_CTL_ELEM_TYPE_INTEGER ((__force >>> snd_ctl_elem_type_t) 2) /* integer type */ >> >> WTF is all that about anyway?? >> What is wrong with: >> #define SNDRV_CTL_ELEM_TYPE_NONE 0u /* invalid */ > > I'm sorry I don't quite understand you. Are you suggesting > SNDRV_CTL_ELEM_TYPE_NONE > isn't needed in the first place? I think David is asking why it's defined the way it is, and I'd guess it's to have the compiler issue an error if you attempt to assign one of these values to a variable or field of the wrong type. No, you should not attempt to change this. -Alex >> David >> >> - >> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, >> MK1 1PT, UK >> Registration No: 1397386 (Wales) >> > > -- > Best regards, > Coiby > _______________________________________________ > greybus-dev mailing list > greybus-dev(a)lists.linaro.org > https://lists.linaro.org/mailman/listinfo/greybus-dev

1 0

Re: [PATCH 3/3] [PATCH] staging: greybus: __u8 is sufficient for snd_ctl_elem_type_t and snd_ctl_elem_iface_t
by Alex Elder 25 Sep '20

25 Sep '20

On 9/25/20 9:07 AM, Coiby Xu wrote: > On Thu, Sep 24, 2020 at 01:00:57PM +0200, Greg Kroah-Hartman wrote: >> On Thu, Sep 24, 2020 at 06:20:39PM +0800, Coiby Xu wrote: >>> Use __8 to replace int and remove the unnecessary __bitwise type >>> attribute. >>> >>> Found by sparse, . . . >>> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h >>> index 535a7229e1d9..8e71a95644ab 100644 >>> --- a/include/uapi/sound/asound.h >>> +++ b/include/uapi/sound/asound.h >>> @@ -950,7 +950,7 @@ struct snd_ctl_card_info { >>> unsigned char components[128]; /* card components / fine >>> identification, delimited with one space (AC97 etc..) */ >>> }; >>> >>> -typedef int __bitwise snd_ctl_elem_type_t; >>> +typedef __u8 snd_ctl_elem_type_t; >>> #define SNDRV_CTL_ELEM_TYPE_NONE ((__force >>> snd_ctl_elem_type_t) 0) /* invalid */ >>> #define SNDRV_CTL_ELEM_TYPE_BOOLEAN ((__force >>> snd_ctl_elem_type_t) 1) /* boolean type */ >>> #define SNDRV_CTL_ELEM_TYPE_INTEGER ((__force >>> snd_ctl_elem_type_t) 2) /* integer type */ >>> @@ -960,7 +960,7 @@ typedef int __bitwise snd_ctl_elem_type_t; >>> #define SNDRV_CTL_ELEM_TYPE_INTEGER64 ((__force >>> snd_ctl_elem_type_t) 6) /* 64-bit integer type */ >>> #define SNDRV_CTL_ELEM_TYPE_LAST SNDRV_CTL_ELEM_TYPE_INTEGER64 >>> >>> -typedef int __bitwise snd_ctl_elem_iface_t; >>> +typedef __u8 snd_ctl_elem_iface_t; >>> #define SNDRV_CTL_ELEM_IFACE_CARD ((__force >>> snd_ctl_elem_iface_t) 0) /* global control */ >>> #define SNDRV_CTL_ELEM_IFACE_HWDEP ((__force >>> snd_ctl_elem_iface_t) 1) /* hardware dependent device */ >>> #define SNDRV_CTL_ELEM_IFACE_MIXER ((__force >>> snd_ctl_elem_iface_t) 2) /* virtual mixer device */ >> >> I can't take a uapi sound header file patch along with a driver change, >> these need to be independant. > > Thank you and Alex for reminding me this is a change of public header! >> >> And this is a userspace api, you just changed the size of an externally >> facing variable, are you _SURE_ that is ok to do? > > The reasons I make this change are, 1) using int to represent 7 enumeration > values seems to be overkill; 2) using one type could avoid worries > about byte order. (1) might be a valid suggestion, but the file you suggest changing is part of the Linux user space API, which you basically can't change. I'm fairly certain the user space API is defined to have CPU-local byte order (unless specified explicitly otherwise) so that is not a concern. > I examine one userspace example (libalsa-intf/alsa_mixer.c [1]). This > change won't cause compiling breakage. But I don't the experience to > imagine any other bad consequence. As a rule, once the user space API has been established, it stays with us forever. You can add to it, but modifying (or removing) an existing interface is essentially forbidden. > Another way to avoid userspace API change is to let > "struct gb_audio_ctl_elem_info" use snd_ctl_elem_iface_t type which may > be more elegant than using "__force" as suggested by Alex, The Greybus definitions were explicitly defined to be operating system independent. For that reason there are (admittedly cumbersome) definitions of certain types and values that essentially mimic those defined by Linux exactly That way Linux (or another system using Greybus) can change its internal values without changing the Greybus API definition. (This addresses the concern you mention below.) What you are suggesting is not consistent with that principle. -Alex > $ git diff > diff --git a/include/linux/greybus/greybus_protocols.h > b/include/linux/greybus/greybus_protocols.h > index aeb8f9243545..7f6753ec7ef7 100644 > --- a/include/linux/greybus/greybus_protocols.h > +++ b/include/linux/greybus/greybus_protocols.h > @@ -8,6 +8,7 @@ > #define __GREYBUS_PROTOCOLS_H > > #include <linux/types.h> > +#include <sound/asound.h> > > /* Fixed IDs for control/svc protocols */ > > @@ -1997,7 +1998,7 @@ struct gb_audio_enumerated { > } __packed; > > struct gb_audio_ctl_elem_info { /* See snd_ctl_elem_info in Linux > source */ > - __u8 type; /* GB_AUDIO_CTL_ELEM_TYPE_* */ > + snd_ctl_elem_type_t type; /* > GB_AUDIO_CTL_ELEM_TYPE_* */ > __le16 dimen[4]; > union { > struct gb_audio_integer integer > > My only concern is if greybus authors have any special reason to not > include > sound/asound.h in the first place and re-define things like > SNDRV_CTL_ELEM_TYPE_*, > > /* See SNDRV_CTL_ELEM_TYPE_* in Linux source */ > #define GB_AUDIO_CTL_ELEM_TYPE_BOOLEAN 0x01 > #define GB_AUDIO_CTL_ELEM_TYPE_INTEGER 0x02 > > /* See SNDRV_CTL_ELEM_IFACE_* in Linux source */ > #define GB_AUDIO_CTL_ELEM_IFACE_CARD 0x00 > ... > > /* SNDRV_CTL_ELEM_ACCESS_* in Linux source */ > #define GB_AUDIO_ACCESS_READ BIT(0) > ... > > [1] > https://gitlab.com/Codeaurora/platform_hardware_qcom_audio/-/blob/aee603282… > > alsa_mixer.c >> >> thanks, >> >> greg k-h > > -- > Best regards, > Coiby

1 0

[PATCH 0/2] regmap: add support to regmap_field_bulk_alloc/free
by Srinivas Kandagatla 25 Sep '20

25 Sep '20

Usage of regmap_field_alloc becomes much overhead when number of fields exceed more than 3. Most of driver seems to totally covered up with these allocs/free making to very hard to read the code! On such driver is QCOM LPASS driver has extensively converted to use regmap_fileds. This patchset add this new api and a user of it. Using new bluk api to allocate fields makes it much more cleaner code to read! Srinivas Kandagatla (2): regmap: add support to regmap_field_bulk_alloc/free apis ASoC: lpass-platform: use devm_regmap_field_bulk_alloc drivers/base/regmap/regmap.c | 100 ++++++++++++++++++++++++++++++++ include/linux/regmap.h | 11 ++++ sound/soc/qcom/lpass-platform.c | 31 +++------- 3 files changed, 118 insertions(+), 24 deletions(-) -- 2.21.0

2 4

alsaloop: Fix the silence generation in xrun_sync
by GitHub pull_request - opened 25 Sep '20

25 Sep '20

alsa-project/alsa-utils pull request #58 was opened from dragosht: The silence frames generation in xrun_sync() seems to be currently a bit off as snd_pcm_format_set_silence() uses a wrong buffer address and size for zero-ing out the data. Consequently instead of clearing out the last frame(s), snd_pcm_format_set_silence() ends up silencing samples somewhere else in the buffer. This is partilarly more obvious for higher frame size (e.g. when using more than 2 channels). This patch fixes this issue by correcting the parameters passed to snd_pcm_format_set_silence(). Signed-off-by: Dragos Tarcatu <dragos_tarcatu(a)mentor.com> Request URL : https://github.com/alsa-project/alsa-utils/pull/58 Patch URL : https://github.com/alsa-project/alsa-utils/pull/58.patch Repository URL: https://github.com/alsa-project/alsa-utils

1 0

[RFC PATCH 1/2] ASoC: cs47l15: Fix EPOUT->HPOUT1 Mono Mux routing
by Richard Fitzgerald 25 Sep '20

25 Sep '20

EPOUT is always mono so should have a permanent routing through the HPOUT1 Mono Mux. Signed-off-by: Richard Fitzgerald <rf(a)opensource.cirrus.com> --- sound/soc/codecs/cs47l15.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/codecs/cs47l15.c b/sound/soc/codecs/cs47l15.c index a591e7457d11..254f9d96e766 100644 --- a/sound/soc/codecs/cs47l15.c +++ b/sound/soc/codecs/cs47l15.c @@ -1089,6 +1089,7 @@ static const struct snd_soc_dapm_route cs47l15_dapm_routes[] = { { "HPOUT1 Demux", NULL, "OUT1R" }, { "OUT1R", NULL, "HPOUT1 Mono Mux" }, + { "HPOUT1 Mono Mux", "EPOUT", "OUT1L" }, { "HPOUTL", "HPOUT", "HPOUT1 Demux" }, { "HPOUTR", "HPOUT", "HPOUT1 Demux" }, @@ -1268,7 +1269,6 @@ static irqreturn_t cs47l15_adsp2_irq(int irq, void *data) static const struct snd_soc_dapm_route cs47l15_mono_routes[] = { { "HPOUT1 Mono Mux", "HPOUT", "OUT1L" }, - { "HPOUT1 Mono Mux", "EPOUT", "OUT1L" }, }; static int cs47l15_component_probe(struct snd_soc_component *component) -- 2.20.1

1 2

[PATCH v2] ALSA: hda - remove kerneldoc for internal hdac_i915 function
by Kai Vehmanen 24 Sep '20

24 Sep '20

Drop the kerneldoc markup for connectivity_check() as it's an static helper function. Fixes the following make W=1 warning: sound/hda/hdac_i915.c:80: warning: Function parameter or member 'i915' not described in 'connectivity_check' sound/hda/hdac_i915.c:80: warning: Function parameter or member 'hdac' not described in 'connectivity_check' Fixes: 7b882fe3e3e8 ('ALSA: hda - handle multiple i915 device instances') Signed-off-by: Kai Vehmanen <kai.vehmanen(a)linux.intel.com> --- sound/hda/hdac_i915.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c index d236e497435d..5f0a1aa6ad84 100644 --- a/sound/hda/hdac_i915.c +++ b/sound/hda/hdac_i915.c @@ -73,9 +73,7 @@ void snd_hdac_i915_set_bclk(struct hdac_bus *bus) } EXPORT_SYMBOL_GPL(snd_hdac_i915_set_bclk); -/** - * Returns true if the devices can be connected for audio. - */ +/* returns true if the devices can be connected for audio */ static bool connectivity_check(struct pci_dev *i915, struct pci_dev *hdac) { struct pci_bus *bus_a = i915->bus, *bus_b = hdac->bus; -- 2.27.0

2 1

[PATCH] ALSA: hda - add missing documentation to hdac_i915
by Kai Vehmanen 24 Sep '20

24 Sep '20

Document missing function arguments and fix make W=1 warning: sound/hda/hdac_i915.c:80: warning: Function parameter or member 'i915' not described in 'connectivity_check' sound/hda/hdac_i915.c:80: warning: Function parameter or member 'hdac' not described in 'connectivity_check' Fixes: 7b882fe3e3e8 ('ALSA: hda - handle multiple i915 device instances') Signed-off-by: Kai Vehmanen <kai.vehmanen(a)linux.intel.com> --- sound/hda/hdac_i915.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c index d236e497435d..9592d0c0c254 100644 --- a/sound/hda/hdac_i915.c +++ b/sound/hda/hdac_i915.c @@ -75,6 +75,8 @@ EXPORT_SYMBOL_GPL(snd_hdac_i915_set_bclk); /** * Returns true if the devices can be connected for audio. + * @i915: i915 PCI device + * @hdac: HDA audio controller PCI device */ static bool connectivity_check(struct pci_dev *i915, struct pci_dev *hdac) { -- 2.27.0

2 3

[PATCH] sound/soc/codecs: fix spelling typo in comments
by Wang Qing 24 Sep '20

24 Sep '20

Modify the comment typo: "compliment" -> "complement". Signed-off-by: Wang Qing <wangqing(a)vivo.com> --- sound/soc/codecs/ak4458.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/codecs/ak4458.h b/sound/soc/codecs/ak4458.h index f906215..e43144c --- a/sound/soc/codecs/ak4458.h +++ b/sound/soc/codecs/ak4458.h @@ -49,7 +49,7 @@ /* DIF2 1 0 * x 1 0 MSB justified Figure 3 (default) - * x 1 1 I2S Compliment Figure 4 + * x 1 1 I2S Complement Figure 4 */ #define AK4458_DIF_SHIFT 1 #define AK4458_DIF_MASK GENMASK(3, 1) -- 2.7.4

3 4

support the Roland UA-4FX2 card
by GitHub issues - opened 24 Sep '20

24 Sep '20

alsa-project/alsa-lib issue #81 was opened from kalashnikov2: hi i'm trying to use this audio card with a Raspberry pi and y get this from the command dmesg on linux console ``` usb 3-2: new high-speed USB device number 6 using xhci_hcd usb 3-2: New USB device found, idVendor=0582, idProduct=01e2, bcdDevice= 0.2b usb 3-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0 usb 3-2: Product: UA-4FX2 usb 3-2: Manufacturer: ROLAND mc: Linux media interface: v0.10 usbcore: registered new interface driver snd-usb-audio usb 3-2: Unable to change format on ep #8e: already in use usb 3-2: Unable to change format on ep #8e: already in use : : ``` and when i try to select the device on a program get this ``` ALSA lib pcm.c:2642:(snd_pcm_open_noupdate) Unknown PCM cards.pcm.rear ALSA lib pcm.c:2642:(snd_pcm_open_noupdate) Unknown PCM cards.pcm.center_lfe ALSA lib pcm.c:2642:(snd_pcm_open_noupdate) Unknown PCM cards.pcm.side ALSA lib pcm_route.c:869:(find_matching_chmap) Found no matching channel map ALSA lib pcm_oss.c:377:(_snd_pcm_oss_open) Unknown field port ALSA lib pcm_oss.c:377:(_snd_pcm_oss_open) Unknown field port ALSA lib pcm_usb_stream.c:486:(_snd_pcm_usb_stream_open) Invalid type for card ALSA lib pcm_usb_stream.c:486:(_snd_pcm_usb_stream_open) Invalid type for card ``` it seems that the card is not supported by ALSA lib. How is the procedure to add new cards to the driver? How can I help to do this? Thank you Issue URL : https://github.com/alsa-project/alsa-lib/issues/81 Repository URL: https://github.com/alsa-project/alsa-lib

2 1