January 2017 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] [PATCH 00/10] ASoC: add OF-graph base simple-card
by Kuninori Morimoto 31 Jan '17

31 Jan '17

Hi Mark, Rob These are OF-graph base simple-card patch-set. Unfortunately, OF-graph DT binding idea for ALSA SoC was rejected many times before. 1st OF-graph idea doesn't have card node, but because of ML discussion, latest idea can have it. OTOH, Mark doesn't like big-patch-set. Thus, some of already accepted preparation patches for OF-graph simple-card was no longer needed. 1) - 4) remove these. This patch set is based on my previous this patch ---------------- Date Thu, 19 Jan 2017 15:31:07 +0900 Subject [PATCH 0/6] of_graph_get_remote_endpoint() ---------------- 1) - 4) : remove soc-core OF-graph adjust feature which is no longer needed 5) - 6) : add useful new of-graph functions 7) - 10) : add OF-graph base simple-card Kuninori Morimoto (10): 1) ASoC: soc-core: remove OF adjusting for snd_soc_of_parse_audio_routing 2) ASoC: soc-core: remove OF adjusting for snd_soc_of_parse_audio_prefix 3) ASoC: soc-core: remove OF adjusting for snd_soc_of_parse_audio_simple_widgets 4) ASoC: soc-core: remove OF adjusting for snd_soc_of_parse_card_name 5) of_graph: add of_graph_get_port_parent() 6) of_graph: add of_graph_get_endpoint_count() 7) ASoC: add snd_soc_get_dai_id() 8) ASoC: simple-card-utils: add asoc_simple_card_parse_graph_dai() 9) ASoC: add simple-graph-card document 10) ASoC: add simple-graph-card support .../bindings/sound/simple-graph-card.txt | 137 ++++++++++ drivers/of/base.c | 42 ++- include/linux/of_graph.h | 14 + include/sound/simple_card_utils.h | 10 + include/sound/soc.h | 31 +-- sound/soc/generic/Kconfig | 7 + sound/soc/generic/Makefile | 2 + sound/soc/generic/simple-card-utils.c | 36 +++ sound/soc/generic/simple-graph-card.c | 303 +++++++++++++++++++++ sound/soc/soc-core.c | 61 +++-- 10 files changed, 588 insertions(+), 55 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/simple-graph-card.txt create mode 100644 sound/soc/generic/simple-graph-card.c -- 1.9.1

2 14

[alsa-devel] License for UCM config files - re plumbers audio conf discussion.
by Liam Girdwood 31 Jan '17

31 Jan '17

During the plumbers audio conference we discussed changing the release cadence and packaging of the UCM config files. This was to separate them from the alsa-lib package and alsa-lib release cadence for easier and more timely distribution for Ubuntu, Fedora etc. One thing that we did not cover was the UCM config file licence, if indeed there is a license for the UCM config files ? It's clear to me that the alsa-lib source and headers are LGPL, but does the LGPL also apply to the runtime UCM config files ? LGPL is specifically designed for libraries so my understanding is that it's probably not applicable to the UCM runtime configuration files. Here the LGPL definition of source code from alsa-lib COPYING :- ""Source code" for a work means the preferred form of the work for making modifications to it. For a library, complete source code means all the source code for all modules it contains, plus any associated interface definition files, plus the scripts used to control compilation and installation of the library." Having a licence free or BSD/MIT type license for the UCM config repo would mean the configurations (or derivatives of) could also be used with Android. IANAL Liam

3 2

[alsa-devel] [Alsa-Devel] Cannot combine audio devices with more than 64 channels
by Jörg Müller 31 Jan '17

31 Jan '17

I need to combine two HDSPe MADI FX cards to one virtual device. I have a working driver, which is alsa-compatible. I can select each single card in any alsa-compatible application and all channels work flawless. For combining the MADI FX cards to one virtual device, I created an .asoundrc with 194 inputs for each card. When I start that virtual device via jackd -R -d alsa -C madifx_record_all -P madifx_playback_all I get this error: > creating alsa driver ... > madifx_playback_all|madifx_record_all|1024|2|48000|0|0|nomon|swmeter|-|32bit > jackd: pcm_multi.c:1060: snd_pcm_multi_open: Assertion `!slave_map[sidxs[i]][schannels[i]]' failed. However, it works when I reduce the amount from 194 to 64 channels per card. I tried to use 128 channels per card, but that fails the same way. See my alsa-info here http://pastebin.com/4hq1B3wZ , which also includes the .asoundrc content. I also found this, which might be related: https://ccrma.stanford.edu/mirrors/lalists/lad/2005/06/0202.html To me, this looks like a bug. What do you think? For reference, I posted the same question on http://lists.jackaudio.org/private.cgi/jack-devel-jackaudio.org/2017-Januar…, however, they recommended to ask here for help.

5 11

[alsa-devel] [PATCH v6 0/3] Add support for es8388 and hdmi audio on the rock2
by Romain Perier 31 Jan '17

31 Jan '17

This set of patches adds a machine driver for rk3288-based boards that have the built-in HDMI audio and a generic analog output. It also adds slave mode to the es8328 driver that currently only supported the master mode. Then, it adds the required DT definitions to link rockchip-i2s to the es8388 analog output and to hdmi audio. This work is based on the initial work that was done by Sjoerd Simons <sjoerd.simons(a)collabora.co.uk> with some improvements, changes and more commits. Changes in v6: - Fixed bad error handling for gpio in rk3288_analog_hdmi.c - Removed message duplication in rk_hw_params(), in rk3288_analog_hdmi.c - Removed patch 1/4 (from the previous series), as it has been merged Changes in v5: - Fixed coding style issue in patch 2/4 (the switch case). - Moved the code for the master mode and the slave mode into the switch case for patch 2/4 - Fixed error handling for patch 3/4 Changes in v4: - Add support for multi codecs in the asoc machine driver, so the driver matches the hw architecture (analog and hdmi audio are connected on the same i2s line) - Renamed the driver to rk3288-hdmi-analog.c and changed its documentation - Add built-in support for hdmi audio in this driver - Add support for the property 'rockchip,routing' Changes in v3: - Cosmetic changes in rockchip_es8388.c - Added missing email to MODULE_AUTHOR in rockchip_es8388.c Changes in v2: - Fixed wrong use of the data structure i2c_device_id - Fixed wrong dependencies for SND_SOC_ROCKCHIP_ES8388 Romain Perier (3): ASoC: es8328: Add support for slave mode ASoC: rockchip: Add machine driver for RK3288 boards that use analog/HDMI arm: dts: Add support for ES8388 to the Radxa Rock 2 .../bindings/sound/rockchip,rk3288-hdmi-analog.txt | 36 +++ arch/arm/boot/dts/rk3288-rock2-square.dts | 39 +++ sound/soc/codecs/es8328.c | 20 +- sound/soc/rockchip/Kconfig | 10 + sound/soc/rockchip/Makefile | 2 + sound/soc/rockchip/rk3288_hdmi_analog.c | 299 +++++++++++++++++++++ 6 files changed, 400 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/rockchip,rk3288-hdmi-analog.txt create mode 100644 sound/soc/rockchip/rk3288_hdmi_analog.c -- 2.9.3

2 5

[alsa-devel] Ceasing ALSA involvement due to persistent ill-treatment
by Jonathan Woithe 31 Jan '17

31 Jan '17

I will keep this brief. When things get to the point where every post one makes to alsa-devel is responded to with personal attacks, disdain, contempt or hostility, where one's extensive practical and technical experience is dismissed as being invalid at every opportunity, and one is consequently anxious about reading the responses to one's posts in alsa-devel, it's clearly time to walk away. Sadly, this is the situation I find myself in so walking away is what I will do once this message has been posted. Moving firewire audio streaming functionality into the kernel has been a long term goal of the FFADO project. Having a decade of experience working successfully with these devices under Linux through FFADO and its predecessors, I was planning to lend assistance to the ALSA firewire streaming driver development effort. Unfortunately, it has become abundantly clear that for some reason this is not welcomed (or even tolerated) by some, so instead I will devote my time to the maintenance of FFADO and my other Open Source projects. Although *extremely* miniscule in comparison to the deplorable harassment directed toward others in the Open Source community in the past, the treatment I have received has given me a fresh perspective on the effect that continued attacks can have on a person's attitude to life, and why many good developers have completely turned their back on the entire Open Source movement when faced with these kinds of issues. The Open Source community has wonderfully awesome people, but it is clear how the misdirected actions of a few can so easily drive people away. I wish the ALSA project well for its future work, and urge project members to treat all participants with the basic human rights of respect and dignity which were not afforded to me over the past two years on alsa-devel. Regards jonathan

3 2

[alsa-devel] Intel alsa sound request for Laptops
by John Frankish 31 Jan '17

31 Jan '17

For two or three kernel versions of alsa divers now, HDMI has been the default alsa device on my laptop (Dell Latitude E7240) - this does not make logical sense. In the latest kernel version I use, 4.2.9, alsa sound will not work at all unless I load the i915 kernel driver - this does not make logical sense and means that I cannot use Xvesa. Is it possible to make an alsa feature request for analogue sound to be the default alsa device on laptops and for alsa sound to work without the need to load the i915 kernel driver? Regards John **** List of PLAYBACK Hardware Devices **** card 0: HDMI [HDA Intel HDMI], device 3: HDMI 0 [HDMI 0] Subdevices: 1/1 Subdevice #0: subdevice #0 card 0: HDMI [HDA Intel HDMI], device 7: HDMI 1 [HDMI 1] Subdevices: 1/1 Subdevice #0: subdevice #0 card 0: HDMI [HDA Intel HDMI], device 8: HDMI 2 [HDMI 2] Subdevices: 1/1 Subdevice #0: subdevice #0 card 1: PCH [HDA Intel PCH], device 0: ALC3226 Analog [ALC3226 Analog] Subdevices: 1/1 Subdevice #0: subdevice #0

4 23

[alsa-devel] workqueue lockup due to process_unsol_events stuck in azx_rirb_get_response
by Vlastimil Babka 31 Jan '17

31 Jan '17

Hi, my desktop randomly experiences workqueue lockups on boot with openSUSE Tumbleweed kernels 4.9.x, installed around Christmas. Previously I had a (badly maintained) Gentoo installation with 4.4 IIRC, so I can't say if the kernel has regressed, or the major userspace changes exposed different timing of stuff. This is how the workqueue lockup looks like: kernel: BUG: workqueue lockup - pool cpus=3 node=0 flags=0x0 nice=0 stuck for 173s! kernel: Showing busy workqueues and worker pools: kernel: workqueue events: flags=0x0 kernel: pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=9/256 kernel: in-flight: 60:process_unsol_events [snd_hda_core] process_unsol_events [snd_hda_core] kernel: pending: dbs_work_handler, push_to_pool, vmstat_shepherd, cache_reap, console_callback, sysrq_reinject_alt_sysrq, check_corrup kernel: workqueue events_power_efficient: flags=0x80 kernel: pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=1/256 kernel: pending: neigh_periodic_work kernel: workqueue lru-add-drain: flags=0x8 kernel: pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=1/256 kernel: pending: lru_add_drain_per_cpu BAR(1161) kernel: workqueue vmstat: flags=0xc kernel: pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=1/256 kernel: pending: vmstat_update kernel: workqueue ipv6_addrconf: flags=0x40008 kernel: pwq 6: cpus=3 node=0 flags=0x0 nice=0 active=1/1 kernel: pending: addrconf_verify_work kernel: pool 6: cpus=3 node=0 flags=0x0 nice=0 hung=173s workers=4 idle: 134 373 30 This prevented me from getting a login prompt, as systemd is waiting for wicked. I have used sysrq to list the stuck tasks: kernel: sysrq: SysRq : Show Blocked State kernel: task PC stack pid father kernel: wickedd-nanny D 0 1161 1 0x00000000 kernel: ffff9f88e3ebc000 0000000000000000 ffff9f88e3506040 ffff9f88efd59780 kernel: ffff9f883ee90100 ffffb2dc81117d28 ffffffffa9715ff6 ffff9f88efd59780 kernel: 00ffb2dc81117d20 ffff9f88efd59780 ffff9f88def44000 ffff9f88e3506040 kernel: Call Trace: kernel: [<ffffffffa9715ff6>] ? __schedule+0x236/0x700 kernel: [<ffffffffa97164fd>] schedule+0x3d/0x90 kernel: [<ffffffffa971984e>] schedule_timeout+0x22e/0x410 kernel: [<ffffffffa90ab6e8>] ? finish_task_switch+0x78/0x1f0 kernel: [<ffffffffa90af94a>] ? try_to_wake_up+0x4a/0x3d0 kernel: [<ffffffffa9717837>] wait_for_completion+0x97/0x100 kernel: [<ffffffffa90afd70>] ? wake_up_q+0x80/0x80 kernel: [<ffffffffa909d747>] flush_work+0xf7/0x190 kernel: [<ffffffffa909b370>] ? flush_workqueue_prep_pwqs+0x1a0/0x1a0 kernel: [<ffffffffa91b4191>] lru_add_drain_all+0x121/0x160 kernel: [<ffffffffa91ddc15>] do_mlock+0x45/0x230 kernel: [<ffffffffa91ddf33>] SyS_mlock+0x13/0x20 kernel: [<ffffffffa971b47b>] entry_SYSCALL_64_fastpath+0x1e/0xad Yep, stuck waiting for work item completion, but workqueue processing on cpu 3 is stuck (and contains such item). Then I listed backtraces of all active CPU's: kernel: sysrq: SysRq : Show backtrace of all active CPUs kernel: NMI backtrace for cpu 3 kernel: CPU: 3 PID: 60 Comm: kworker/3:1 Not tainted 4.9.3-1-default #1 kernel: Hardware name: Gigabyte Technology Co., Ltd. GA-870A-UD3/GA-870A-UD3, BIOS F5 08/01/2011 kernel: Workqueue: events process_unsol_events [snd_hda_core] kernel: ffff9f88efcc3a90 ffffffffa93c4f70 0000000000000000 0000000000000003 kernel: ffff9f88efcc3ac0 ffffffffa93c8e44 ffffffffa9055c80 0000000000000003 kernel: 0000000000000001 ffffffffa9cc43c0 ffff9f88efcc3ae0 ffffffffa93c8f66 kernel: Call Trace: kernel: <IRQ> kernel: [<ffffffffa93c4f70>] dump_stack+0x63/0x83 kernel: [<ffffffffa93c8e44>] nmi_cpu_backtrace+0x94/0xa0 kernel: [<ffffffffa9055c80>] ? irq_force_complete_move+0x150/0x150 kernel: [<ffffffffa93c8f66>] nmi_trigger_cpumask_backtrace+0x116/0x150 kernel: [<ffffffffa9055cf9>] arch_trigger_cpumask_backtrace+0x19/0x20 kernel: [<ffffffffa94ce877>] sysrq_handle_showallcpus+0x17/0x20 kernel: [<ffffffffa94cef1b>] __handle_sysrq+0xfb/0x150 [ snip keyboard event processing ] kernel: [<ffffffffa903128d>] handle_irq+0x1d/0x30 kernel: [<ffffffffa971ddab>] do_IRQ+0x4b/0xd0 kernel: [<ffffffffa971be02>] common_interrupt+0x82/0x82 kernel: <EOI> kernel: [<ffffffffa93d1ef3>] ? delay_tsc+0x43/0x90 kernel: [<ffffffffa93d1e2d>] __const_udelay+0x2d/0x30 kernel: [<ffffffffc0c08403>] azx_rirb_get_response+0xb3/0x280 [snd_hda_codec] kernel: [<ffffffffc0c08b43>] azx_get_response+0x33/0x40 [snd_hda_codec] kernel: [<ffffffffc0b86303>] snd_hdac_bus_exec_verb_unlocked+0x83/0x170 [snd_hda_core] kernel: [<ffffffffa952e85c>] ? __pm_runtime_resume+0x4c/0x60 kernel: [<ffffffffc0bfe4fa>] codec_exec_verb+0x8a/0x110 [snd_hda_codec] kernel: [<ffffffffc0b86f97>] snd_hdac_exec_verb+0x17/0x40 [snd_hda_core] kernel: [<ffffffffc0b87944>] snd_hdac_codec_read+0x34/0x50 [snd_hda_core] kernel: [<ffffffffc0c20501>] dspio_read+0x51/0x70 [snd_hda_codec_ca0132] kernel: [<ffffffffc0c20566>] ca0132_process_dsp_response+0x46/0x160 [snd_hda_codec_ca0132] kernel: [<ffffffffc0c02fe5>] call_jack_callback.isra.1+0x25/0xa0 [snd_hda_codec] kernel: [<ffffffffc0c033c6>] snd_hda_jack_unsol_event+0x66/0x80 [snd_hda_codec] kernel: [<ffffffffc0bfd077>] hda_codec_unsol_event+0x17/0x20 [snd_hda_codec] kernel: [<ffffffffc0b86193>] process_unsol_events+0x63/0x70 [snd_hda_core] kernel: [<ffffffffa909e4d3>] process_one_work+0x1f3/0x4d0 kernel: [<ffffffffa909e7f8>] worker_thread+0x48/0x4e0 kernel: [<ffffffffa909e7b0>] ? process_one_work+0x4d0/0x4d0 kernel: [<ffffffffa909e7b0>] ? process_one_work+0x4d0/0x4d0 kernel: [<ffffffffa90a46da>] kthread+0xca/0xe0 kernel: [<ffffffffa90a4610>] ? kthread_park+0x60/0x60 kernel: [<ffffffffa971b6f5>] ret_from_fork+0x25/0x30 This backtrace looks stable on multiple retries. I've rebooted and added module_blacklist=snd_hda_core on the command line to work around this for now. Here's the relevant lspci. The card I use right now is an onboard audio with 3 output jacks for 5.1. The Creative Audigy is currently unused, as should be the HDMI stuff. 00:14.2 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] SBx00 Azalia (Intel HDA) (rev 40) Subsystem: Gigabyte Technology Co., Ltd Device a102 Flags: bus master, slow devsel, latency 32, IRQ 7, NUMA node 0 Memory at fe024000 (64-bit, non-prefetchable) [size=16K] Capabilities: [50] Power Management version 2 Kernel modules: snd_hda_intel 01:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Cape Verde/Pitcairn HDMI Audio [Radeon HD 7700/7800 Series] Subsystem: PC Partner Limited / Sapphire Technology Device aab0 Flags: bus master, fast devsel, latency 0, IRQ 10, NUMA node 0 Memory at fd4fc000 (64-bit, non-prefetchable) [size=16K] Capabilities: [48] Vendor Specific Information: Len=08 <?> Capabilities: [50] Power Management version 3 Capabilities: [58] Express Legacy Endpoint, MSI 00 Capabilities: [a0] MSI: Enable- Count=1/1 Maskable- 64bit+ Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1 Len=010 <?> Capabilities: [150] Advanced Error Reporting Kernel modules: snd_hda_intel 02:00.0 Audio device: Creative Labs Sound Core3D [Sound Blaster Recon3D / Z-Series] (rev 01) Subsystem: Creative Labs SB1570 SB Audigy Fx Flags: bus master, fast devsel, latency 0, IRQ 10, NUMA node 0 Memory at fd3fc000 (64-bit, non-prefetchable) [size=16K] Memory at fd3f8000 (64-bit, non-prefetchable) [size=16K] Capabilities: [40] Power Management version 3 Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+ Capabilities: [70] Express Endpoint, MSI 00 Capabilities: [100] Advanced Error Reporting Capabilities: [140] Virtual Channel Capabilities: [170] Device Serial Number 00-00-00-00-00-00-00-00 Capabilities: [180] Power Budgeting <?> Kernel modules: snd_hda_intel Let me know what else to dump or try. Thanks, Vlastimil

2 5

[alsa-devel] [PATCH 1/2] ASoC: qcom: lpass-cpu: Remove unnecessary clock checks
by Bjorn Andersson 30 Jan '17

30 Jan '17

Clean up the clock calling code by removing numerous IS_ERR() checks by just assigning the clock NULL; as this turn all used functions in the clk API to nops. Also include the word "optional" in the error message when failing to acquire the optional osr clocks. Cc: Banajit Goswami <bgoswami(a)codeaurora.org> Cc: Patrick Lai <plai(a)codeaurora.org> Cc: Srinivas Kandagatla <srinivas.kandagatla(a)linaro.org> Signed-off-by: Bjorn Andersson <bjorn.andersson(a)linaro.org> --- sound/soc/qcom/lpass-cpu.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c index eff3f9a8b685..1b912a9bb791 100644 --- a/sound/soc/qcom/lpass-cpu.c +++ b/sound/soc/qcom/lpass-cpu.c @@ -33,9 +33,6 @@ static int lpass_cpu_daiops_set_sysclk(struct snd_soc_dai *dai, int clk_id, struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai); int ret; - if (IS_ERR(drvdata->mi2s_osr_clk[dai->driver->id])) - return 0; - ret = clk_set_rate(drvdata->mi2s_osr_clk[dai->driver->id], freq); if (ret) dev_err(dai->dev, "%s() error setting mi2s osrclk to %u: %d\n", @@ -50,23 +47,18 @@ static int lpass_cpu_daiops_startup(struct snd_pcm_substream *substream, struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai); int ret; - if (!IS_ERR(drvdata->mi2s_osr_clk[dai->driver->id])) { - ret = clk_prepare_enable( - drvdata->mi2s_osr_clk[dai->driver->id]); - if (ret) { - dev_err(dai->dev, "%s() error in enabling mi2s osr clk: %d\n", - __func__, ret); - return ret; - } + ret = clk_prepare_enable(drvdata->mi2s_osr_clk[dai->driver->id]); + if (ret) { + dev_err(dai->dev, "%s() error in enabling mi2s osr clk: %d\n", + __func__, ret); + return ret; } ret = clk_prepare_enable(drvdata->mi2s_bit_clk[dai->driver->id]); if (ret) { dev_err(dai->dev, "%s() error in enabling mi2s bit clk: %d\n", __func__, ret); - if (!IS_ERR(drvdata->mi2s_osr_clk[dai->driver->id])) - clk_disable_unprepare( - drvdata->mi2s_osr_clk[dai->driver->id]); + clk_disable_unprepare(drvdata->mi2s_osr_clk[dai->driver->id]); return ret; } @@ -80,8 +72,7 @@ static void lpass_cpu_daiops_shutdown(struct snd_pcm_substream *substream, clk_disable_unprepare(drvdata->mi2s_bit_clk[dai->driver->id]); - if (!IS_ERR(drvdata->mi2s_osr_clk[dai->driver->id])) - clk_disable_unprepare(drvdata->mi2s_osr_clk[dai->driver->id]); + clk_disable_unprepare(drvdata->mi2s_osr_clk[dai->driver->id]); } static int lpass_cpu_daiops_hw_params(struct snd_pcm_substream *substream, @@ -505,9 +496,11 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) clk_name); if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) { dev_warn(&pdev->dev, - "%s() error getting mi2s-osr-clk: %ld\n", + "%s() error getting optional mi2s-osr-clk: %ld\n", __func__, PTR_ERR(drvdata->mi2s_osr_clk[dai_id])); + + drvdata->mi2s_osr_clk[dai_id] = NULL; } if (variant->num_dai > 1) -- 2.11.0

1 1

Re: [alsa-devel] [PATCH v2 1/1] ALSA: Tascam US-16x08 DSP mixer quirk
by Takashi Iwai 30 Jan '17

30 Jan '17

On Mon, 30 Jan 2017 14:20:18 +0100, Detlef Urban wrote: > > I bet on "global variables have been removed". Good to hear, but I'm not sure. See below. > But still unclear to me is if use of private > data field in struct usb_mixer_elem_info is > uncommon? It's normal to use it. > Add mixer quirk for Tascam US-16x08 usb interface. > Even that this an usb compliant device, > the input channels and DSP functions (EQ/Compressor) > arn't accessible by default. > > Signed-off-by: Detlef Urban <onkel(a)paraair.de> Well, the patch style looks still very incompatible. Please run checkpatch.pl and fix the issues reported there at first. Also, if it's your MUA that breaks the spacing of the patch, then please fix your MUA setup, or use attachment as a last resort. About the non-coding-style issues in the patch: > +int snd_us16x08_recv_urb(struct snd_usb_audio *chip, > + unsigned char *buf, int size) Missing static? > +int snd_us16x08_recv_urb_0xa1(struct snd_usb_audio *chip, > + char *buffer, int size) Ditto. I won't mention in all others. Please check each function whether it really must be non-static or not. > +{ > + > + unsigned char buf[64] = {0,}; > + int err = snd_usb_ctl_msg(chip->dev, > + usb_rcvctrlpipe(chip->dev, 0), > + 2, > + 0xa1, 0x0100, 0x100, buf, 14); > + dev_err(&chip->dev->dev, > + "us16x08: snd_us16x08_recv_urb: error: %d\n", err); Is the intention to show always the error? If yes, please give some more comments to the function itself. > + > + return 0; > +} > + > +/* wrapper function to send prepared URB buffer to usb device. Return -1 > + * if something went wrong (FIXME: have to have more helpful error numbers) Better to fix now :) > + */ > +int snd_us16x08_send_urb(struct snd_usb_audio *chip, char *buf, int size) .... > +struct meter_resp { > + uint8_t a0; > + uint8_t a1; > + uint8_t a2; > + uint8_t b0; > + uint8_t b1; > + uint8_t b2; > + uint8_t c0; > + uint8_t c1; > + uint8_t c2; > + uint8_t c3; > +}; > +struct meter_resp *resp; > +unsigned char meter_urb_buf[64] = {0,}; What about these are global...? I thought you bet it. > +int snd_us16x08_eqswitch_info(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_info *uinfo) > +{ > + uinfo->count = 1; > + uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN; > + uinfo->value.integer.max = 1; > + uinfo->value.integer.min = 0; > + return 0; > +} Use the standard snd_ctl_boolean_mono_info(). > +static int snd_us16x08_eqswitch_put(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + struct usb_mixer_elem_info *elem = kcontrol->private_data; > + struct snd_usb_audio *chip = elem->head.mixer->chip; > + struct snd_us16x08_eq_all_store *store = > + ((struct snd_us16x08_eq_all_store *) elem->private_data); > + int index = ucontrol->id.index; > + > + char buf[sizeof(eqs_msq)]; > + int val, err = 0; > + > + val = ucontrol->value.integer.value[0]; > + > + memcpy(buf, eqs_msq, sizeof(eqs_msq)); > + > + store->low_store->valSwitch[index] = val; > + store->midlow_store->valSwitch[index] = val; > + store->midhigh_store->valSwitch[index] = val; > + store->high_store->valSwitch[index] = val; > + > + buf[5] = index + 1; > + > + buf[20] = store->low_store->valSwitch[index]; > + buf[17] = store->low_store->valWidth[index]; > + buf[14] = store->low_store->valFreq[index]; > + buf[11] = store->low_store->valdB[index]; > + buf[8] = 0x01; > + err = snd_us16x08_send_urb(chip, buf, sizeof(eqs_msq)); > + > + mdelay(15); Need to be mdelay(), not msleep() or variant? > +struct snd_us16x08_fader *snd_us16x08_create_mix_store(int default_val) > +{ > + int i; > + struct snd_us16x08_fader *tmp = > + kmalloc(sizeof(struct snd_us16x08_fader), GFP_KERNEL); Missing NULL check. > + > + for (i = 0; i < SND_US16X08_MAX_CHANNELS; i++) > + tmp->value[i] = default_val; > + return tmp; > +} > + > +struct snd_us16x08_comp_store *snd_us16x08_create_comp_store(void) > +{ > + int i = 0; > + struct snd_us16x08_comp_store *tmp = > + kmalloc(sizeof(struct snd_us16x08_comp_store), GFP_KERNEL); Ditto. > + > + for (i = 0; i < SND_US16X08_MAX_CHANNELS; i++) { > + tmp->valThreshold[i] = 0x20; /* 0dB */ > + tmp->valRatio[i] = 0x00; /* 1:1 */ > + tmp->valGain[i] = 0x00; /* 0dB */ > + tmp->valSwitch[i] = 0x00; /* on */ > + tmp->valAttack[i] = 0x02; /* 2ms */ > + tmp->valRelease[i] = 0x01; /* 10ms */ > + } > + return tmp; > +} > + > +struct snd_us16x08_bypass_store *snd_us16x08_create_single_store( > + int default_val) > +{ > + struct snd_us16x08_bypass_store *tmp = > + kmalloc(sizeof(struct snd_us16x08_bypass_store), GFP_KERNEL); Ditto... For the whole! > +void snd_us16x08_free_eq_store(struct snd_kcontrol *kctl) > +{ > + snd_usb_mixer_elem_free(kctl); > +} > + > +void snd_us16x08_free_mix_store(struct snd_kcontrol *kctl) > +{ > + snd_usb_mixer_elem_free(kctl); > +} Why two different functions for the very same thing? > +static int snd_us16x08_meter_get(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + int i, set; > + int val; > + struct usb_mixer_elem_info *elem = kcontrol->private_data; > + struct snd_usb_audio *chip = elem->head.mixer->chip; > + struct snd_us16x08_meter_store *store = elem->private_data; > + /* struct snd_us16x08_comp_store *comp_store; */ > + > + if (elem) { > + store = (struct snd_us16x08_meter_store *) elem->private_data; > + chip = elem->head.mixer->chip; > + } > + > + switch (kcontrol->private_value) { > + case 0: > + snd_us16x08_send_urb(chip, mix_init_msg1, 4); > + snd_us16x08_recv_urb(chip, meter_urb_buf, > + sizeof(meter_urb_buf)); > + kcontrol->private_value++; > + break; > + case 1: > + snd_us16x08_recv_urb(chip, meter_urb_buf, > + sizeof(meter_urb_buf)); > + kcontrol->private_value++; > + break; > + case 2: > + snd_us16x08_recv_urb(chip, meter_urb_buf, > + sizeof(meter_urb_buf)); > + kcontrol->private_value++; > + break; > + case 3: > + mix_init_msg2[2] = snd_get_meter_comp_index(store); > + snd_us16x08_send_urb(chip, mix_init_msg2, 10); > + snd_us16x08_recv_urb(chip, meter_urb_buf, > + sizeof(meter_urb_buf)); > + kcontrol->private_value = 0; > + break; > + } ..... WTF does this function do...? Please add more comments. > +int snd_us16x08_controls_create_eq(struct usb_mixer_interface *mixer) > +{ > + > + int err; > + char name[64]; > + struct usb_mixer_elem_info *elem; > + > + struct snd_us16x08_eq_store *eq_low_store = > + snd_us16x08_create_eq_store(0x01); > + struct snd_us16x08_eq_store *eq_midlow_store = > + snd_us16x08_create_eq_store(0x02); > + struct snd_us16x08_eq_store *eq_midhigh_store = > + snd_us16x08_create_eq_store(0x03); > + struct snd_us16x08_eq_store *eq_high_store = > + snd_us16x08_create_eq_store(0x04); > + struct snd_us16x08_eq_all_store *eq_all_store = > + kmalloc(sizeof(struct snd_us16x08_eq_all_store), GFP_KERNEL); > + > + eq_all_store->low_store = eq_low_store; > + eq_all_store->midlow_store = eq_midlow_store; > + eq_all_store->midhigh_store = eq_midhigh_store; > + eq_all_store->high_store = eq_high_store; You didn't check NULL at all here... > + snprintf(name, sizeof(name), "5 Low"); > + err = add_new_ctl(mixer, &snd_us16x08_eqlevel_ctl, > + SND_US16X08_ID_EQLOWLEVEL, 0x00, 0, USB_MIXER_U8, 1, name, > + (void *) eq_low_store, snd_us16x08_free_eq_store, &elem); > + if (err < 0) > + return err; > + > + snprintf(name, sizeof(name), "51 LowFreq"); .... Please use a table for this kind of code. > +int snd_us16x08_controls_create(struct usb_mixer_interface *mixer) > +{ > + > + int err; > + char name[64]; > + struct usb_mixer_elem_info *elem; > + struct snd_us16x08_fader *route_store; > + struct snd_us16x08_comp_store *comp_store; > + struct snd_us16x08_meter_store *meter_store; > + > + if (mixer->chip->num_interfaces > 0) { What is this? > + route_store = snd_us16x08_create_mix_store(0); Missing error check. > + route_store->value[0] = 0; > + route_store->value[1] = 1; > + route_store->value[2] = 4; > + route_store->value[3] = 5; > + route_store->value[4] = 6; > + route_store->value[5] = 7; > + route_store->value[6] = 8; > + route_store->value[7] = 9; > + snprintf(name, sizeof(name), "Route"); > + err = add_new_ctl(mixer, &snd_us16x08_route_ctl, > + 0x00, 0x00, 0, > + USB_MIXER_U8, 1, name, (void *) route_store, > + snd_us16x08_free_mix_store, &elem); > + if (err < 0) > + return err; > + > + snprintf(name, sizeof(name), "Master"); ..... Please use a table. > --- /dev/null > +++ b/sound/usb/mixer_us16x08.h > @@ -0,0 +1,132 @@ > +#ifndef __USB_MIXER_US16X08_H > +#define __USB_MIXER_US16X08_H > + > + > +#define SND_US16X08_MIN_CHANNELS 0 Zero channel is allowed...? > +#define SND_US16X08_MAX_CHANNELS 16 > + > +/* set macro for kcontrol private_value */ > +#define SND_US16X08_KCSET(bias, step, min, max) \ > + ((bias << 24) | (step << 16) | (min << 8) | max) Put parentheses to each argument in a macro. > +struct snd_us16x08_fader { > + uint8_t value[SND_US16X08_MAX_CHANNELS]; Use u8 for the kernel code. > +struct snd_us16x08_comp_store { > + int8_t valThreshold[SND_US16X08_MAX_CHANNELS]; > + uint8_t valAttack[SND_US16X08_MAX_CHANNELS]; > + uint8_t valRelease[SND_US16X08_MAX_CHANNELS]; > + uint8_t valRatio[SND_US16X08_MAX_CHANNELS]; > + uint8_t valGain[SND_US16X08_MAX_CHANNELS]; > + uint8_t valSwitch[SND_US16X08_MAX_CHANNELS]; > +}; > + > +struct snd_us16x08_meter_store { > + long meter_level[SND_US16X08_MAX_CHANNELS]; > + long master_level[2]; /* level of meter for master output */ The size of long depends on the architecture. Do you want 32bit or 64bit integer? For the latter, use s64 or u64. For the former, just use the standard int. > + int comp_index; > + int comp_active_index; > + long comp_level[16]; /* compressor reduction level of current > channel */ Ditto. > + struct snd_us16x08_comp_store *comp_store; > +}; > + > +struct snd_us16x08_bypass_store { > + int value; > +}; What's the merit of this struct? > +typedef void(*opt_free)(struct snd_kcontrol *kctl); Don't use typedef unless really required. > +int snd_us16x08_mix_info(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_info *uinfo); > + > +int snd_us16x08_controls_create(struct usb_mixer_interface *mixer); > + > +int snd_us16x08_send_urb(struct snd_usb_audio *chip, char *buf, int size); > + > +int snd_us16x08_cur_channel(void); > + > +int add_new_ctl(struct usb_mixer_interface *mixer, > + const struct snd_kcontrol_new *ncontrol, > + int index, int offset, int num, int val_type, int channels, > + const char *name, const void *opt, > + opt_free freeer, struct usb_mixer_elem_info **elem_ret); > + > +int snd_us16x08_controls_create_eq(struct usb_mixer_interface *mixer); > +int snd_us16x08_controls_create_comp(struct usb_mixer_interface *mixer); > + > +struct snd_us16x08_comp_store *get_comp_store(void); Where is it defined? Also, did you test suspend/resume? Your code seems missing the restore mixer callback. Without the proper restore callback, S3 may work, but S4 won't. thanks, Takashi

1 0

[alsa-devel] [PATCH] Sound: soc: samsung - Fix possible NULL derefrence.
by Shailendra Verma 30 Jan '17

30 Jan '17

of_device_get_match_data could return NULL, and so can cause a NULL pointer dereference later. Signed-off-by: Shailendra Verma <shailendra.v(a)samsung.com> --- sound/soc/samsung/i2s.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c index 7825bff..b5acce2 100644 --- a/sound/soc/samsung/i2s.c +++ b/sound/soc/samsung/i2s.c @@ -1224,8 +1224,13 @@ static int samsung_i2s_probe(struct platform_device *pdev) const struct samsung_i2s_dai_data *i2s_dai_data; int ret; - if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) + if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) { i2s_dai_data = of_device_get_match_data(&pdev->dev); + if (!i2s_dai_data) { + dev_err(&pdev->dev, "no device match found\n"); + return -ENODEV; + } + } else i2s_dai_data = (struct samsung_i2s_dai_data *) platform_get_device_id(pdev)->driver_data; -- 1.7.9.5

2 1