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

[alsa-devel] [PATCH] tegra_wm9712: Fix a memory leaking bug in tegra_wm9712_driver_probe()
by Gen Zhang 24 May '19

24 May '19

In tegra_wm9712_driver_probe(), 'machine->codec' is allocated by platform_device_alloc(). When it is NULL, function returns ENOMEM. However, 'machine' is allocated by devm_kzalloc() before this site. Thus we should free 'machine' before function ends to prevent memory leaking. Further, we should free 'machine->util_data', 'machine->codec' and 'machine' before this function normally ends to prevent memory leaking. Signed-off-by: Gen Zhang <blackgod016574(a)gmail.com> --- diff --git a/sound/soc/tegra/tegra_wm9712.c b/sound/soc/tegra/tegra_wm9712.c index 864a334..295c41d 100644 --- a/sound/soc/tegra/tegra_wm9712.c +++ b/sound/soc/tegra/tegra_wm9712.c @@ -86,7 +86,8 @@ static int tegra_wm9712_driver_probe(struct platform_device *pdev) machine->codec = platform_device_alloc("wm9712-codec", -1); if (!machine->codec) { dev_err(&pdev->dev, "Can't allocate wm9712 platform device\n"); - return -ENOMEM; + ret = -ENOMEM; + goto codec_free; } ret = platform_device_add(machine->codec); @@ -127,6 +128,10 @@ static int tegra_wm9712_driver_probe(struct platform_device *pdev) goto asoc_utils_fini; } + tegra_asoc_utils_fini(&machine->util_data); + platform_device_del(machine->codec); + platform_device_put(machine->codec); + devm_kfree(&pdev->dev, machine); return 0; asoc_utils_fini: @@ -135,6 +140,8 @@ static int tegra_wm9712_driver_probe(struct platform_device *pdev) platform_device_del(machine->codec); codec_put: platform_device_put(machine->codec); +codec_free: + devm_kfree(&pdev->dev, machine); return ret; } ---

2 6

Re: [alsa-devel] [PATCH 2/5] ASoC: madera: Add common support for Cirrus Logic Madera codecs
by Charles Keepax 24 May '19

24 May '19

On Fri, May 24, 2019 at 03:56:03PM +0100, Mark Brown wrote: > On Fri, May 24, 2019 at 11:41:55AM +0100, Charles Keepax wrote: > > > + /* > > + * Just read a register a few times to ensure the internal > > + * oscillator sends out a few clocks. > > + */ > > + for (i = 0; i < 4; i++) { > > + ret = regmap_read(madera->regmap, MADERA_SOFTWARE_RESET, &val); > > + if (ret) > > + dev_err(madera->dev, > > + "%s Failed to read register: %d (%d)\n", > > + __func__, ret, i); > > Why use %s to format the __func__ rather than just concatenate? > Really the error message is quite out of character in general, we don't normally print the func and it doesn't really say what failed. I will refactor it. > > + } > > + > > + udelay(300); > > So we read the register a few times then add a few hundred us of delay > after? Surely that delay is going to be negligable compared to the time > spent on I/O? > This is one of those technically situations. We have to do the reads and we have to have at least 300uS after those. Normally there will be enough slack in the system but better to guarantee it. Maybe a comment? > > +int madera_sysclk_ev(struct snd_soc_dapm_widget *w, > > + struct snd_kcontrol *kcontrol, int event) > > +{ > > + struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm); > > + struct madera_priv *priv = snd_soc_component_get_drvdata(component); > > + > > + madera_spin_sysclk(priv); > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(madera_sysclk_ev); > > This will delay both before and after every power up and power down. > Are you sure that makes sense? > Regrettably yes, or well at least I am sure that despite a lot of complaining this is what the hardware guys insisted needed done. > > + > > + ret = madera_check_speaker_overheat(madera, &warn, &shutdown); > > + if (ret) > > + shutdown = true; /* for safety attempt to shutdown on error */ > > + > > + if (shutdown) { > > + dev_crit(madera->dev, "Thermal shutdown\n"); > > + ret = regmap_update_bits(madera->regmap, > > + MADERA_OUTPUT_ENABLES_1, > > + MADERA_OUT4L_ENA | > > + MADERA_OUT4R_ENA, 0); > > + if (ret != 0) > > + dev_crit(madera->dev, > > + "Failed to disable speaker outputs: %d\n", > > + ret); > > + } else if (warn) { > > + dev_crit(madera->dev, "Thermal warning\n"); > > + } > > + > > + return IRQ_HANDLED; > > We will flag the interrupt as handled if there was neither a warning nor > a critical overheat? I'd expect some warning about a spurious interrupt > at least. > Yeah I will update that one. > > +static int madera_get_variable_u32_array(struct madera_priv *priv, > > + const char *propname, > > + u32 *dest, > > + int n_max, > > + int multiple) > > +{ > > + struct madera *madera = priv->madera; > > + int n, ret; > > + > > + n = device_property_read_u32_array(madera->dev, propname, NULL, 0); > > + if (n == -EINVAL) { > > + return 0; /* missing, ignore */ > > + } else if (n < 0) { > > + dev_warn(madera->dev, "%s malformed (%d)\n", > > + propname, n); > > + return n; > > + } else if ((n % multiple) != 0) { > > + dev_warn(madera->dev, "%s not a multiple of %d entries\n", > > + propname, multiple); > > + return -EINVAL; > > + } > > + > > + if (n > n_max) > > + n = n_max; > > + > > + ret = device_property_read_u32_array(madera->dev, propname, dest, n); > > + > > + if (ret < 0) > > + return ret; > > + else > > + return n; > > +} > > This feels like it should perhaps be a generic OF helper function - > there's nothing driver specific I'm seeing here and arrays that need to > be a multiple of N entries aren't that uncommon I think. > I will have a look and see if we might do a generic helper function for this one. > > + mutex_lock(&priv->rate_lock); > > + cached_rate = priv->adsp_rate_cache[adsp_num]; > > + mutex_unlock(&priv->rate_lock); > > What's this lock protecting? The value can we read can change as soon > as the lock is released and we're just reading a single word here rather > than traversing a data structure that might change under us or > something. > Overall the lock protects anything that changes a rate on the chip, since we have to do that weird spin_sysclk thing before/after anything that does that. It might normally be safe without the lock here I guess, since you are only really protecting against the reads of adsp_rate_cache. But suppose that might depend on the architecture if the read/writes arn't effectively atomic (although that is probably unlikely on most modern architectures). > > +void madera_destroy_bus_error_irq(struct madera_priv *priv, int dsp_num) > > +{ > > + struct madera *madera = priv->madera; > > + > > + madera_free_irq(madera, > > + madera_dsp_bus_error_irqs[dsp_num], > > + &priv->adsp[dsp_num]); > > +} > > +EXPORT_SYMBOL_GPL(madera_destroy_bus_error_irq); > > We use free rather than destroy normally? > Yup will fix that. > > +static const char * const madera_dfc_width_text[MADERA_DFC_WIDTH_ENUM_SIZE] = { > > + "8bit", "16bit", "20bit", "24bit", "32bit", > > +}; > > Spaces might make these more readable. > No problem to fix that too. > > +static void madera_sleep(unsigned int delay) > > +{ > > + if (delay < 20) { > > + delay *= 1000; > > + usleep_range(delay, delay + 500); > > + } else { > > + msleep(delay); > > + } > > +} > > This feels like it might make sense as a helper function as well - I > could've sworn there was one already but I can't immediately find it. I do vaguely remember some discussion that it was preferred that people had to think about which delay function to call, rather than having a generic helper. TBF I could probably just switch it all to use msleep, for the most part this is a quite small optimisation of path bring up (although it was asked for by one of our customers). Thanks, Charles

1 0

Re: [alsa-devel] [PATCH 2/5] ASoC: madera: Add common support for Cirrus Logic Madera codecs
by Richard Fitzgerald 24 May '19

24 May '19

On 24/05/19 15:56, Mark Brown wrote: > On Fri, May 24, 2019 at 11:41:55AM +0100, Charles Keepax wrote: > >> + /* >> + * Just read a register a few times to ensure the internal >> + * oscillator sends out a few clocks. >> + */ >> + for (i = 0; i < 4; i++) { >> + ret = regmap_read(madera->regmap, MADERA_SOFTWARE_RESET, &val); >> + if (ret) >> + dev_err(madera->dev, >> + "%s Failed to read register: %d (%d)\n", >> + __func__, ret, i); > > Why use %s to format the __func__ rather than just concatenate? GCC docs say that it's a magic variable so cannot be concatenated with string literals. Though I never tried concatenation to see if it works. > >> + } >> + >> + udelay(300); > > So we read the register a few times then add a few hundred us of delay > after? Surely that delay is going to be negligable compared to the time > spent on I/O? The register reads are to create clock cycles in the silicon, not to generate delay. > >> +int madera_sysclk_ev(struct snd_soc_dapm_widget *w, >> + struct snd_kcontrol *kcontrol, int event) >> +{ >> + struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm); >> + struct madera_priv *priv = snd_soc_component_get_drvdata(component); >> + >> + madera_spin_sysclk(priv); >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(madera_sysclk_ev); > > This will delay both before and after every power up and power down. > Are you sure that makes sense? > I think that's correct but we can re-check with hardware people. It's not just a delay, it needs to ensure there are always a certain number of SYSCLK cycles in the hardware to avoid leaving certain state machines in limbo. >> + >> + ret = madera_check_speaker_overheat(madera, &warn, &shutdown); >> + if (ret) >> + shutdown = true; /* for safety attempt to shutdown on error */ >> + >> + if (shutdown) { >> + dev_crit(madera->dev, "Thermal shutdown\n"); >> + ret = regmap_update_bits(madera->regmap, >> + MADERA_OUTPUT_ENABLES_1, >> + MADERA_OUT4L_ENA | >> + MADERA_OUT4R_ENA, 0); >> + if (ret != 0) >> + dev_crit(madera->dev, >> + "Failed to disable speaker outputs: %d\n", >> + ret); >> + } else if (warn) { >> + dev_crit(madera->dev, "Thermal warning\n"); >> + } >> + >> + return IRQ_HANDLED; > > We will flag the interrupt as handled if there was neither a warning nor > a critical overheat? I'd expect some warning about a spurious interrupt > at least. > >> +static int madera_get_variable_u32_array(struct madera_priv *priv, >> + const char *propname, >> + u32 *dest, >> + int n_max, >> + int multiple) >> +{ >> + struct madera *madera = priv->madera; >> + int n, ret; >> + >> + n = device_property_read_u32_array(madera->dev, propname, NULL, 0); >> + if (n == -EINVAL) { >> + return 0; /* missing, ignore */ >> + } else if (n < 0) { >> + dev_warn(madera->dev, "%s malformed (%d)\n", >> + propname, n); >> + return n; >> + } else if ((n % multiple) != 0) { >> + dev_warn(madera->dev, "%s not a multiple of %d entries\n", >> + propname, multiple); >> + return -EINVAL; >> + } >> + >> + if (n > n_max) >> + n = n_max; >> + >> + ret = device_property_read_u32_array(madera->dev, propname, dest, n); >> + >> + if (ret < 0) >> + return ret; >> + else >> + return n; >> +} > > This feels like it should perhaps be a generic OF helper function - > there's nothing driver specific I'm seeing here and arrays that need to > be a multiple of N entries aren't that uncommon I think. > >> + mutex_lock(&priv->rate_lock); >> + cached_rate = priv->adsp_rate_cache[adsp_num]; >> + mutex_unlock(&priv->rate_lock); > > What's this lock protecting? The value can we read can change as soon > as the lock is released and we're just reading a single word here rather > than traversing a data structure that might change under us or > something. > >> +void madera_destroy_bus_error_irq(struct madera_priv *priv, int dsp_num) >> +{ >> + struct madera *madera = priv->madera; >> + >> + madera_free_irq(madera, >> + madera_dsp_bus_error_irqs[dsp_num], >> + &priv->adsp[dsp_num]); >> +} >> +EXPORT_SYMBOL_GPL(madera_destroy_bus_error_irq); > > We use free rather than destroy normally? > >> +static const char * const madera_dfc_width_text[MADERA_DFC_WIDTH_ENUM_SIZE] = { >> + "8bit", "16bit", "20bit", "24bit", "32bit", >> +}; > > Spaces might make these more readable. > >> +static void madera_sleep(unsigned int delay) >> +{ >> + if (delay < 20) { >> + delay *= 1000; >> + usleep_range(delay, delay + 500); >> + } else { >> + msleep(delay); >> + } >> +} > > This feels like it might make sense as a helper function as well - I > could've sworn there was one already but I can't immediately find it. >

1 1

[alsa-devel] [PATCH v2] ASoC: drop COMPILE_TEST for sound/soc/intel/boards/ that use X86_INTEL_LPSS
by Randy Dunlap 24 May '19

24 May '19

From: Randy Dunlap <rdunlap(a)infradead.org> This is a partial revert of 164a263bf8d0, which causes build errors on non-X86 platforms (specifically seen on IA64) when COMPILE_TEST is set/enabled. Fixes these build errors (on ia64): i../sound/soc/intel/boards/bxt_da7219_max98357a.c:19:10: fatal error: asm/cpu_device_id.h: No such file or directory #include <asm/cpu_device_id.h> ../sound/soc/intel/boards/bytcr_rt5640.c:31:10: fatal error: asm/cpu_device_id.h: No such file or directory #include <asm/cpu_device_id.h> ../sound/soc/intel/boards/bytcr_rt5651.c:33:10: fatal error: asm/cpu_device_id.h: No such file or directory #include <asm/cpu_device_id.h> ../sound/soc/intel/boards/cht_bsw_rt5645.c:29:10: fatal error: asm/cpu_device_id.h: No such file or directory #include <asm/cpu_device_id.h> ../sound/soc/intel/boards/bytcht_es8316.c:33:10: fatal error: asm/cpu_device_id.h: No such file or directory #include <asm/cpu_device_id.h> ../sound/soc/intel/boards/bytcht_da7213.c:26:10: fatal error: asm/platform_sst_audio.h: No such file or directory #include <asm/platform_sst_audio.h> Fixes: 164a263bf8d0 ("ASoC: Intel: Make boards more available for compile test") Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org> Cc: Mark Brown <broonie(a)kernel.org> Cc: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Cc: Liam Girdwood <liam.r.girdwood(a)linux.intel.com> Cc: Jie Yang <yang.jie(a)linux.intel.com> Cc: alsa-devel(a)alsa-project.org --- sound/soc/intel/boards/Kconfig | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) --- lnx-52-rc1.orig/sound/soc/intel/boards/Kconfig +++ lnx-52-rc1/sound/soc/intel/boards/Kconfig @@ -18,7 +18,7 @@ config SND_SOC_INTEL_HASWELL_MACH tristate "Haswell Lynxpoint" depends on I2C depends on I2C_DESIGNWARE_PLATFORM || COMPILE_TEST - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_RT5640 help This adds support for the Lynxpoint Audio DSP on Intel(R) Haswell @@ -35,7 +35,7 @@ config SND_SOC_INTEL_BDW_RT5677_MACH depends on I2C depends on I2C_DESIGNWARE_PLATFORM || COMPILE_TEST depends on GPIOLIB || COMPILE_TEST - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_RT5677 help This adds support for Intel Broadwell platform based boards with @@ -47,7 +47,7 @@ config SND_SOC_INTEL_BROADWELL_MACH tristate "Broadwell Wildcatpoint" depends on I2C depends on I2C_DESIGNWARE_PLATFORM || COMPILE_TEST - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_RT286 help This adds support for the Wilcatpoint Audio DSP on Intel(R) Broadwell @@ -61,7 +61,7 @@ if SND_SOC_INTEL_BAYTRAIL config SND_SOC_INTEL_BYT_MAX98090_MACH tristate "Baytrail with MAX98090 codec" depends on I2C - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_MAX98090 help This adds audio driver for Intel Baytrail platform based boards @@ -72,7 +72,7 @@ config SND_SOC_INTEL_BYT_MAX98090_MACH config SND_SOC_INTEL_BYT_RT5640_MACH tristate "Baytrail with RT5640 codec" depends on I2C - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_RT5640 help This adds audio driver for Intel Baytrail platform based boards @@ -86,7 +86,7 @@ if SND_SST_ATOM_HIFI2_PLATFORM || SND_SO config SND_SOC_INTEL_BYTCR_RT5640_MACH tristate "Baytrail and Baytrail-CR with RT5640 codec" depends on I2C && ACPI - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_ACPI select SND_SOC_RT5640 help @@ -98,7 +98,7 @@ config SND_SOC_INTEL_BYTCR_RT5640_MACH config SND_SOC_INTEL_BYTCR_RT5651_MACH tristate "Baytrail and Baytrail-CR with RT5651 codec" depends on I2C && ACPI - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_ACPI select SND_SOC_RT5651 help @@ -110,7 +110,7 @@ config SND_SOC_INTEL_BYTCR_RT5651_MACH config SND_SOC_INTEL_CHT_BSW_RT5672_MACH tristate "Cherrytrail & Braswell with RT5672 codec" depends on I2C && ACPI - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_ACPI select SND_SOC_RT5670 help @@ -122,7 +122,7 @@ config SND_SOC_INTEL_CHT_BSW_RT5672_MACH config SND_SOC_INTEL_CHT_BSW_RT5645_MACH tristate "Cherrytrail & Braswell with RT5645/5650 codec" depends on I2C && ACPI - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_ACPI select SND_SOC_RT5645 help @@ -134,7 +134,7 @@ config SND_SOC_INTEL_CHT_BSW_RT5645_MACH config SND_SOC_INTEL_CHT_BSW_MAX98090_TI_MACH tristate "Cherrytrail & Braswell with MAX98090 & TI codec" depends on I2C && ACPI - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_MAX98090 select SND_SOC_TS3A227E help @@ -146,7 +146,7 @@ config SND_SOC_INTEL_CHT_BSW_MAX98090_TI config SND_SOC_INTEL_CHT_BSW_NAU8824_MACH tristate "Cherrytrail & Braswell with NAU88L24 codec" depends on I2C && ACPI - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_ACPI select SND_SOC_NAU8824 help @@ -158,7 +158,7 @@ config SND_SOC_INTEL_CHT_BSW_NAU8824_MAC config SND_SOC_INTEL_BYT_CHT_DA7213_MACH tristate "Baytrail & Cherrytrail with DA7212/7213 codec" depends on I2C && ACPI - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_ACPI select SND_SOC_DA7213 help @@ -170,7 +170,7 @@ config SND_SOC_INTEL_BYT_CHT_DA7213_MACH config SND_SOC_INTEL_BYT_CHT_ES8316_MACH tristate "Baytrail & Cherrytrail with ES8316 codec" depends on I2C && ACPI - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS select SND_SOC_ACPI select SND_SOC_ES8316 help @@ -186,7 +186,7 @@ if SND_SST_ATOM_HIFI2_PLATFORM config SND_SOC_INTEL_BYT_CHT_NOCODEC_MACH tristate "Baytrail & Cherrytrail platform with no codec (MinnowBoard MAX, Up)" depends on I2C && ACPI - depends on X86_INTEL_LPSS || COMPILE_TEST + depends on X86_INTEL_LPSS help This adds support for ASoC machine driver for the MinnowBoard Max or Up boards and provides access to I2S signals on the Low-Speed @@ -248,7 +248,7 @@ if SND_SOC_INTEL_APL config SND_SOC_INTEL_BXT_DA7219_MAX98357A_MACH tristate "Broxton with DA7219 and MAX98357A in I2S Mode" depends on I2C && ACPI - depends on MFD_INTEL_LPSS || COMPILE_TEST + depends on MFD_INTEL_LPSS select SND_SOC_DA7219 select SND_SOC_MAX98357A select SND_SOC_DMIC

2 1

[alsa-devel] [PATCH] ASoc: fix sound/soc/intel/skylake/slk-ssp-clk.c build error on IA64
by Randy Dunlap 24 May '19

24 May '19

From: Randy Dunlap <rdunlap(a)infradead.org> skl-ssp-clk.c does not build on IA64 because the driver uses the common clock interface, so make the driver depend on COMMON_CLK. Fixes this build error: ../sound/soc/intel/skylake/skl-ssp-clk.c:26:16: error: field 'hw' has incomplete type struct clk_hw hw; ^~ Reported-by: kbuild test robot <lkp(a)intel.com> Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org> Cc: Mark Brown <broonie(a)kernel.org> Cc: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Cc: Liam Girdwood <liam.r.girdwood(a)linux.intel.com> Cc: Jie Yang <yang.jie(a)linux.intel.com> Cc: alsa-devel(a)alsa-project.org --- sound/soc/intel/Kconfig | 3 ++- sound/soc/intel/boards/Kconfig | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) --- lnx-52-rc1.orig/sound/soc/intel/Kconfig +++ lnx-52-rc1/sound/soc/intel/Kconfig @@ -104,7 +104,7 @@ config SND_SST_ATOM_HIFI2_PLATFORM_ACPI config SND_SOC_INTEL_SKYLAKE tristate "All Skylake/SST Platforms" depends on PCI && ACPI - select SND_SOC_INTEL_SKL + select SND_SOC_INTEL_SKL if COMMON_CLK select SND_SOC_INTEL_APL select SND_SOC_INTEL_KBL select SND_SOC_INTEL_GLK @@ -120,6 +120,7 @@ config SND_SOC_INTEL_SKYLAKE config SND_SOC_INTEL_SKL tristate "Skylake Platforms" depends on PCI && ACPI + depends on COMMON_CLK select SND_SOC_INTEL_SKYLAKE_FAMILY help If you have a Intel Skylake platform with the DSP enabled --- lnx-52-rc1.orig/sound/soc/intel/boards/Kconfig +++ lnx-52-rc1/sound/soc/intel/boards/Kconfig @@ -286,7 +286,7 @@ config SND_SOC_INTEL_KBL_RT5663_MAX98927 select SND_SOC_MAX98927 select SND_SOC_DMIC select SND_SOC_HDAC_HDMI - select SND_SOC_INTEL_SKYLAKE_SSP_CLK + select SND_SOC_INTEL_SKYLAKE_SSP_CLK if COMMON_CLK help This adds support for ASoC Onboard Codec I2S machine driver. This will create an alsa sound card for RT5663 + MAX98927.

2 3

[alsa-devel] [PATCH] Revert "ASoC: simple-card: Fix configuration of DAI format"
by Jon Hunter 24 May '19

24 May '19

Revert commit 069d037aea98 ("ASoC: simple-card: Fix configuration of DAI format"). During further review, it turns out that the actual issue was caused by an incorrectly formatted device-tree node describing the soundcard. The following is incorrect because the simple-audio-card 'bitclock-master' and 'frame-master' properties should not reference the actual codec phandle ... sound { compatible = "simple-audio-card"; ... => simple-audio-card,bitclock-master = <&codec>; => simple-audio-card,frame-master = <&codec>; ... simple-audio-card,cpu { sound-dai = <&xxx>; }; simple-audio-card,codec { => sound-dai = <&codec>; }; }; Rather, these properties should reference the phandle to the 'simple-audio-card,codec' property as shown below ... sound { compatible = "simple-audio-card"; ... => simple-audio-card,bitclock-master = <&codec>; => simple-audio-card,frame-master = <&codec>; ... simple-audio-card,cpu { sound-dai = <&xxx>; }; => codec: simple-audio-card,codec { /* simple-card wants here */ sound-dai = <&xxx>; /* not here */ }; }; Signed-off-by: Jon Hunter <jonathanh(a)nvidia.com> --- sound/soc/generic/simple-card.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index c2a2c5fd0801..d16e894fce2b 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -283,6 +283,11 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv, codec_dai = dai_props->codec_dai = &priv->dais[li->dais++]; + ret = asoc_simple_parse_daifmt(dev, node, codec, + prefix, &dai_link->dai_fmt); + if (ret < 0) + goto dai_link_of_err; + simple_parse_mclk_fs(top, cpu, codec, dai_props, prefix); ret = asoc_simple_parse_cpu(cpu, dai_link, &single_cpu); @@ -293,11 +298,6 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv, if (ret < 0) goto dai_link_of_err; - ret = asoc_simple_parse_daifmt(dev, node, dai_link->codecs->of_node, - prefix, &dai_link->dai_fmt); - if (ret < 0) - goto dai_link_of_err; - ret = asoc_simple_parse_platform(plat, dai_link); if (ret < 0) goto dai_link_of_err; -- 2.7.4

2 1

[alsa-devel] [PATCH 1/5] ASoC: madera: Add DT bindings for Cirrus Logic Madera codecs
by Charles Keepax 24 May '19

24 May '19

From: Richard Fitzgerald <rf(a)opensource.cirrus.com> The Cirrus Logic Madera codecs are a family of related codecs with extensive digital and analogue I/O, digital mixing and routing, signal processing and programmable DSPs. Signed-off-by: Richard Fitzgerald <rf(a)opensource.cirrus.com> Signed-off-by: Charles Keepax <ckeepax(a)opensource.cirrus.com> --- Documentation/devicetree/bindings/sound/madera.txt | 67 ++++++++++++++++++++++ MAINTAINERS | 1 + include/dt-bindings/sound/madera.h | 29 ++++++++++ 3 files changed, 97 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/madera.txt create mode 100644 include/dt-bindings/sound/madera.h diff --git a/Documentation/devicetree/bindings/sound/madera.txt b/Documentation/devicetree/bindings/sound/madera.txt new file mode 100644 index 0000000000000..1114fcf1aa4c2 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/madera.txt @@ -0,0 +1,67 @@ +Cirrus Logic Madera class audio codecs + +This describes audio configuration bindings for these codecs. + +See also the core bindings for the parent MFD driver: +See Documentation/devicetree/bindings/mfd/madera.txt + +and defines for values used in these bindings: +include/dt-bindings/sound/madera.h + +These properties are all contained in the parent MFD node. + +Optional properties: + - cirrus,dmic-ref : Indicates how the MICBIAS pins have been externally + connected to DMICs on each input, one cell per input. + <IN1 IN2 IN3 ...> + A value of 0 indicates MICVDD and is the default, other values depend on the + codec: + For CS47L35 one of the CS47L35_DMIC_REF_xxx values + For all other codecs one of the MADERA_DMIC_REF_xxx values + Also see the datasheet for a description of the INn_DMIC_SUP field. + + - cirrus,inmode : A list of input mode settings for each input. A maximum of + 16 cells, with four cells per input in the order INnAL, INnAR INnBL INnBR. + For non-muxed inputs the first two cells for that input set the mode for + the left and right channel and the second two cells must be 0. + For muxed inputs the first two cells for that input set the mode of the + left and right A inputs and the second two cells set the mode of the left + and right B inputs. + Valid mode values are one of the MADERA_INMODE_xxx. If the array is shorter + than the number of inputs the unspecified inputs default to + MADERA_INMODE_DIFF. + + - cirrus,out-mono : Mono bit for each output, must contain six cells if + specified. A non-zero value indicates the corresponding output is mono. + + - cirrus,max-channels-clocked : Maximum number of channels that I2S clocks + will be generated for. Useful when clock master for systems where the I2S + bus has multiple data lines. + One cell for each AIF, use a value of zero for AIFs that should be handled + normally. + + - cirrus,pdm-fmt : PDM speaker data format, must contain 2 cells + (OUT5 and OUT6). See the PDM_SPKn_FMT field in the datasheet for a + description of this value. + The second cell is ignored for codecs that do not have OUT6. + + - cirrus,pdm-mute : PDM mute format, must contain 2 cells + (OUT5 and OUT6). See the PDM_SPKn_CTRL_1 register in the datasheet for a + description of this value. + The second cell is ignored for codecs that do not have OUT6. + +Example: + +cs47l35@0 { + compatible = "cirrus,cs47l35"; + + cirrus,dmic-ref = <0 0 CS47L35_DMIC_REF_MICBIAS1B 0>; + cirrus,inmode = < + MADERA_INMODE_DMIC MADERA_INMODE_DMIC /* IN1A digital */ + MADERA_INMODE_SE MADERA_INMODE_SE /* IN1B single-ended */ + MADERA_INMODE_DIFF MADERA_INMODE_DIFF /* IN2 differential */ + 0 0 /* not used on this codec */ + >; + cirrus,out-mono = <0 0 0 0 0 0>; + cirrus,max-channels-clocked = <2 0 0>; +}; diff --git a/MAINTAINERS b/MAINTAINERS index 5cfbea4ce5750..642cb5610dd50 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3915,6 +3915,7 @@ W: https://github.com/CirrusLogic/linux-drivers/wiki S: Supported F: Documentation/devicetree/bindings/mfd/madera.txt F: Documentation/devicetree/bindings/pinctrl/cirrus,madera-pinctrl.txt +F: include/dt-bindings/sound/madera* F: include/linux/irqchip/irq-madera* F: include/linux/mfd/madera/* F: drivers/gpio/gpio-madera* diff --git a/include/dt-bindings/sound/madera.h b/include/dt-bindings/sound/madera.h new file mode 100644 index 0000000000000..9ff4eae5259b0 --- /dev/null +++ b/include/dt-bindings/sound/madera.h @@ -0,0 +1,29 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Device Tree defines for Madera codecs + * + * Copyright (C) 2016-2017 Cirrus Logic, Inc. and + * Cirrus Logic International Semiconductor Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef DT_BINDINGS_SOUND_MADERA_H +#define DT_BINDINGS_SOUND_MADERA_H + +#define MADERA_INMODE_DIFF 0 +#define MADERA_INMODE_SE 1 +#define MADERA_INMODE_DMIC 2 + +#define MADERA_DMIC_REF_MICVDD 0 +#define MADERA_DMIC_REF_MICBIAS1 1 +#define MADERA_DMIC_REF_MICBIAS2 2 +#define MADERA_DMIC_REF_MICBIAS3 3 + +#define CS47L35_DMIC_REF_MICBIAS1B 1 +#define CS47L35_DMIC_REF_MICBIAS2A 2 +#define CS47L35_DMIC_REF_MICBIAS2B 3 + +#endif -- 2.11.0

1 4

[alsa-devel] [PATCH] pcm030-audio-fabric: Fix a memory leaking bug in pcm030_fabric_probe()
by Gen Zhang 24 May '19

24 May '19

In pcm030_fabric_probe(), 'pdata->codec_device' is allocated by platform_device_alloc(). When this allocation fails, ENOMEM is returned. However, 'pdata' is allocated by devm_kzalloc() before this site. We should free 'pdata' before function ends to prevent memory leaking. Similarly, we should free 'pdata' when 'pdata->codec_device' is NULL. And we should free 'pdata->codec_device' and 'pdata' when 'ret' is error to prevent memory leaking. Signed-off-by: Gen Zhang <blackgod016574(a)gmail.com> --- diff --git a/sound/soc/fsl/pcm030-audio-fabric.c b/sound/soc/fsl/pcm030-audio-fabric.c index a7fe4ad..d2e6eae 100644 --- a/sound/soc/fsl/pcm030-audio-fabric.c +++ b/sound/soc/fsl/pcm030-audio-fabric.c @@ -72,29 +72,43 @@ static int pcm030_fabric_probe(struct platform_device *op) platform_np = of_parse_phandle(np, "asoc-platform", 0); if (!platform_np) { dev_err(&op->dev, "ac97 not registered\n"); - return -ENODEV; + ret = -ENODEV; + goto out_free1; } for_each_card_prelinks(card, i, dai_link) dai_link->platform_of_node = platform_np; ret = request_module("snd-soc-wm9712"); - if (ret) + if (ret) { dev_err(&op->dev, "request_module returned: %d\n", ret); + goto out_free1; + } pdata->codec_device = platform_device_alloc("wm9712-codec", -1); - if (!pdata->codec_device) + if (!pdata->codec_device) { dev_err(&op->dev, "platform_device_alloc() failed\n"); + ret = -ENOMEM; + goto out_free1; + } ret = platform_device_add(pdata->codec_device); - if (ret) + if (ret) { dev_err(&op->dev, "platform_device_add() failed: %d\n", ret); + goto out_free2; + } ret = snd_soc_register_card(card); - if (ret) + if (ret) { dev_err(&op->dev, "snd_soc_register_card() failed: %d\n", ret); + goto out_free2; + } platform_set_drvdata(op, pdata); +out_free2: + platform_device_put(pdata->codec_device); +out_free1: + devm_kfree(&op->dev, pdata); return ret; } ---

2 1

[alsa-devel] [PATCH 0/4] ALSA: firewire-lib: bug fix and code cleanup for packet streaming engine
by Takashi Sakamoto 24 May '19

24 May '19

Hi, This patchset is the last part of refactoring for ALSA IEC 61883-1/6 packet streaming engine. It includes two fixes and two refactoring. Takashi Sakamoto (4): ALSA: firewire-lib: fix data block counter for incoming packet without CIP header ALSA: firewire-lib: fix inverted node IDs for amdtp_packet events ALSA: firewire-lib: refactoring to obsolete IT packet handler ALSA: firewire-lib: refactoring to obsolete IR packet handler sound/firewire/amdtp-stream-trace.h | 6 +- sound/firewire/amdtp-stream.c | 138 ++++++++++++++-------------- 2 files changed, 74 insertions(+), 70 deletions(-) -- 2.20.1

2 5

[alsa-devel] [PATCH 0/2] add pcm_file support in MMAP read access mode
by Adam Miartus 24 May '19

24 May '19

adds support for overwritting input buffer in MMAP access mode for in pcm_file plugin new api pcm_fast_ops callback mmap_begin is introduced to ease the operation needed to overwrite mmaped buffer. this allows to modify only the next continuous part of data in ring buffer. Adam Miartus (2): pcm: add mmap_begin callback to snd_pcm_fast_ops_t api pcm_file: add infile read support for mmap mode src/pcm/pcm.c | 6 ++++++ src/pcm/pcm_file.c | 31 +++++++++++++++++++++++++++++++ src/pcm/pcm_local.h | 1 + 3 files changed, 38 insertions(+) -- 2.7.4

2 3