June 2022 - Alsa-devel - mailman.alsa-project.org

[PATCH] ASoC: tegra: Fix MBDRC bypass mode check
by Sameer Pujar 15 Jun '22

15 Jun '22

MBDRC supports different modes of operation. There is no configuration required for bypass mode. The hw_params() call does not filter bypass mode correctly and it leads to following Smatch static checker warning: sound/soc/tegra/tegra210_mbdrc.c:778 tegra210_mbdrc_hw_params() warn: bitwise AND condition is false here Fix this condition by using proper mode mask and just return for bypass mode. Reported-by: Dan Carpenter <dan.carpenter(a)oracle.com> Fixes: 7358a803c778 ("ASoC: tegra: Add Tegra210 based OPE driver") Signed-off-by: Sameer Pujar <spujar(a)nvidia.com> --- sound/soc/tegra/tegra210_mbdrc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sound/soc/tegra/tegra210_mbdrc.c b/sound/soc/tegra/tegra210_mbdrc.c index 7d9da33..d786daa 100644 --- a/sound/soc/tegra/tegra210_mbdrc.c +++ b/sound/soc/tegra/tegra210_mbdrc.c @@ -775,7 +775,9 @@ int tegra210_mbdrc_hw_params(struct snd_soc_component *cmpnt) regmap_read(ope->mbdrc_regmap, TEGRA210_MBDRC_CFG, &val); - if (val & TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS) + val &= TEGRA210_MBDRC_CFG_MBDRC_MODE_MASK; + + if (val == TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS) return 0; for (i = 0; i < MBDRC_NUM_BAND; i++) { -- 2.7.4

2 1

[PATCH v2 0/2] Add pinctrl support adsp based platforms
by Srinivasa Rao Mandadapu 15 Jun '22

15 Jun '22

This patch set is to add pinctrl support adsp enabled sc7280 platforms. Changes Since V1: -- Update commit message. Srinivasa Rao Mandadapu (2): dt-bindings: pinctrl: qcom: sc7280: Add compatible string for adsp based platforms pinctrl: qcom: sc7280: Add lpi pinctrl variant data for adsp based targets .../bindings/pinctrl/qcom,sc7280-lpass-lpi-pinctrl.yaml | 4 +++- drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) -- 2.7.4

3 8

[v1] ASoC: rockchip: i2s: switch BCLK to GPIO
by Judy Hsiao 15 Jun '22

15 Jun '22

We discoverd that the state of BCLK on, LRCLK off and SD_MODE on may cause the speaker melting issue. Removing LRCLK while BCLK is present can cause unexpected output behavior including a large DC output voltage as described in the Max98357a datasheet. In order to: 1. prevent BCLK from turning on by other component. 2. keep BCLK and LRCLK being present at the same time This patch switches BCLK to GPIO func before LRCLK output, and configures BCLK func back during LRCLK is output. Without this fix, BCLK is turned on 11 ms earlier than LRCK by the da7219. With this fix, BCLK is turned on only 0.4 ms earlier than LRCK by the rockchip codec. Signed-off-by: Judy Hsiao <judyhsiao(a)chromium.org> --- sound/soc/rockchip/rockchip_i2s.c | 162 ++++++++++++++++++++++++------ 1 file changed, 130 insertions(+), 32 deletions(-) diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 47a3971a9ce1..cecaadd54b48 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -13,6 +13,7 @@ #include <linux/of_gpio.h> #include <linux/of_device.h> #include <linux/clk.h> +#include <linux/pinctrl/consumer.h> #include <linux/pm_runtime.h> #include <linux/regmap.h> #include <linux/spinlock.h> @@ -54,8 +55,40 @@ struct rk_i2s_dev { const struct rk_i2s_pins *pins; unsigned int bclk_ratio; spinlock_t lock; /* tx/rx lock */ + struct pinctrl *pinctrl; + struct pinctrl_state *bclk_on; + struct pinctrl_state *bclk_off; }; +static int i2s_pinctrl_select_bclk_on(struct rk_i2s_dev *i2s) +{ + int ret = 0; + + if (!IS_ERR(i2s->pinctrl) && !IS_ERR_OR_NULL(i2s->bclk_on)) + ret = pinctrl_select_state(i2s->pinctrl, + i2s->bclk_on); + + if (ret) + dev_err(i2s->dev, "bclk enable failed %d\n", ret); + + return ret; +} + +static int i2s_pinctrl_select_bclk_off(struct rk_i2s_dev *i2s) +{ + + int ret = 0; + + if (!IS_ERR(i2s->pinctrl) && !IS_ERR_OR_NULL(i2s->bclk_off)) + ret = pinctrl_select_state(i2s->pinctrl, + i2s->bclk_off); + + if (ret) + dev_err(i2s->dev, "bclk disable failed %d\n", ret); + + return ret; +} + static int i2s_runtime_suspend(struct device *dev) { struct rk_i2s_dev *i2s = dev_get_drvdata(dev); @@ -92,38 +125,49 @@ static inline struct rk_i2s_dev *to_info(struct snd_soc_dai *dai) return snd_soc_dai_get_drvdata(dai); } -static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) +static int rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) { unsigned int val = 0; int retry = 10; + int ret = 0; spin_lock(&i2s->lock); if (on) { - regmap_update_bits(i2s->regmap, I2S_DMACR, - I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE); + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, + I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE); + if (ret < 0) + goto end; - regmap_update_bits(i2s->regmap, I2S_XFER, - I2S_XFER_TXS_START | I2S_XFER_RXS_START, - I2S_XFER_TXS_START | I2S_XFER_RXS_START); + ret = regmap_update_bits(i2s->regmap, I2S_XFER, + I2S_XFER_TXS_START | I2S_XFER_RXS_START, + I2S_XFER_TXS_START | I2S_XFER_RXS_START); + if (ret < 0) + goto end; i2s->tx_start = true; } else { i2s->tx_start = false; - regmap_update_bits(i2s->regmap, I2S_DMACR, - I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE); + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, + I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE); + if (ret < 0) + goto end; if (!i2s->rx_start) { - regmap_update_bits(i2s->regmap, I2S_XFER, - I2S_XFER_TXS_START | - I2S_XFER_RXS_START, - I2S_XFER_TXS_STOP | - I2S_XFER_RXS_STOP); + ret = regmap_update_bits(i2s->regmap, I2S_XFER, + I2S_XFER_TXS_START | + I2S_XFER_RXS_START, + I2S_XFER_TXS_STOP | + I2S_XFER_RXS_STOP); + if (ret < 0) + goto end; udelay(150); - regmap_update_bits(i2s->regmap, I2S_CLR, - I2S_CLR_TXC | I2S_CLR_RXC, - I2S_CLR_TXC | I2S_CLR_RXC); + ret = regmap_update_bits(i2s->regmap, I2S_CLR, + I2S_CLR_TXC | I2S_CLR_RXC, + I2S_CLR_TXC | I2S_CLR_RXC); + if (ret < 0) + goto end; regmap_read(i2s->regmap, I2S_CLR, &val); @@ -138,44 +182,57 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) } } } - spin_unlock(&i2s->lock); +end: + spin_unlock(&i2s->lock); + if (ret < 0) + dev_err(i2s->dev, "lrclk update failed\n"); + + return ret; } -static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) +static int rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) { unsigned int val = 0; int retry = 10; + int ret = 0; spin_lock(&i2s->lock); if (on) { - regmap_update_bits(i2s->regmap, I2S_DMACR, + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_ENABLE); + if (ret < 0) + goto end; - regmap_update_bits(i2s->regmap, I2S_XFER, + ret = regmap_update_bits(i2s->regmap, I2S_XFER, I2S_XFER_TXS_START | I2S_XFER_RXS_START, I2S_XFER_TXS_START | I2S_XFER_RXS_START); + if (ret < 0) + goto end; i2s->rx_start = true; } else { i2s->rx_start = false; - regmap_update_bits(i2s->regmap, I2S_DMACR, + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_DISABLE); + if (ret < 0) + goto end; if (!i2s->tx_start) { - regmap_update_bits(i2s->regmap, I2S_XFER, + ret = regmap_update_bits(i2s->regmap, I2S_XFER, I2S_XFER_TXS_START | I2S_XFER_RXS_START, I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); - + if (ret < 0) + goto end; udelay(150); - regmap_update_bits(i2s->regmap, I2S_CLR, + ret = regmap_update_bits(i2s->regmap, I2S_CLR, I2S_CLR_TXC | I2S_CLR_RXC, I2S_CLR_TXC | I2S_CLR_RXC); - + if (ret < 0) + goto end; regmap_read(i2s->regmap, I2S_CLR, &val); - /* Should wait for clear operation to finish */ while (val) { regmap_read(i2s->regmap, I2S_CLR, &val); @@ -187,7 +244,12 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) } } } +end: spin_unlock(&i2s->lock); + if (ret < 0) + dev_err(i2s->dev, "lrclk update failed\n"); + + return ret; } static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai, @@ -425,17 +487,26 @@ static int rockchip_i2s_trigger(struct snd_pcm_substream *substream, case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) - rockchip_snd_rxctrl(i2s, 1); + ret = rockchip_snd_rxctrl(i2s, 1); else - rockchip_snd_txctrl(i2s, 1); + ret = rockchip_snd_txctrl(i2s, 1); + /* Do not turn on bclk if lrclk open fails. */ + if (ret < 0) + return ret; + i2s_pinctrl_select_bclk_on(i2s); break; case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) - rockchip_snd_rxctrl(i2s, 0); - else - rockchip_snd_txctrl(i2s, 0); + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { + if (!i2s->tx_start) + i2s_pinctrl_select_bclk_off(i2s); + ret = rockchip_snd_rxctrl(i2s, 0); + } else { + if (!i2s->rx_start) + i2s_pinctrl_select_bclk_off(i2s); + ret = rockchip_snd_txctrl(i2s, 0); + } break; default: ret = -EINVAL; @@ -736,6 +807,33 @@ static int rockchip_i2s_probe(struct platform_device *pdev) } i2s->bclk_ratio = 64; + i2s->pinctrl = devm_pinctrl_get(&pdev->dev); + if (IS_ERR(i2s->pinctrl)) + dev_err(&pdev->dev, "failed to find i2s pinctrl\n"); + + i2s->bclk_on = pinctrl_lookup_state(i2s->pinctrl, + "bclk_on"); + if (IS_ERR_OR_NULL(i2s->bclk_on)) + dev_err(&pdev->dev, "failed to find i2s default state\n"); + else + dev_dbg(&pdev->dev, "find i2s bclk state\n"); + + i2s->bclk_off = pinctrl_lookup_state(i2s->pinctrl, + "bclk_off"); + if (IS_ERR_OR_NULL(i2s->bclk_off)) + dev_err(&pdev->dev, "failed to find i2s gpio state\n"); + else + dev_dbg(&pdev->dev, "find i2s bclk_off state\n"); + + i2s_pinctrl_select_bclk_off(i2s); + + i2s->playback_dma_data.addr = res->start + I2S_TXDR; + i2s->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; + i2s->playback_dma_data.maxburst = 4; + + i2s->capture_dma_data.addr = res->start + I2S_RXDR; + i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; + i2s->capture_dma_data.maxburst = 4; dev_set_drvdata(&pdev->dev, i2s); -- 2.36.1.476.g0c4daa206d-goog

2 1

ACPI REV override for Dell XPS 13 9343
by Cezary Rojewski 15 Jun '22

15 Jun '22

Hello, It's been a while since catpt-driver [1] has been introduced to provide full support for Broadwell (BDW) machines with Intel DSP. For BDW, audio devices can make use of DSP only in I2S mode. In 2015 Rafael and Dominik provided quirk [2] for Dell XPS 13 9343. Given the description: _For example, based on what ACPI exports as the supported revision, Dell XPS 13 (2015) configures its audio device to either work in HDA mode or in I2S mode, where the former is supposed to be used on Linux until the latter is fully supported (in the kernel as well as in user space)._ It's clear that such configuration was not fully supported back then. I believe now it is. Perhaps it is time to let the quirk in mention go? By that I mean just the relevant entry, not the ACPI_REV_OVERRIDE_POSSIBLE functionality as a whole. [1]: https://lore.kernel.org/all/20200929141247.8058-1-cezary.rojewski@intel.com/ [2]: https://lore.kernel.org/alsa-devel/5115175.tbCl1JQVaV@vostro.rjw.lan/ Regards, Czarek

3 5

[bug report] ASoC: tegra: Add Tegra210 based OPE driver
by Dan Carpenter 15 Jun '22

15 Jun '22

Hello Sameer Pujar, The patch 7358a803c778: "ASoC: tegra: Add Tegra210 based OPE driver" from Jun 3, 2022, leads to the following Smatch static checker warning: sound/soc/tegra/tegra210_mbdrc.c:778 tegra210_mbdrc_hw_params() warn: bitwise AND condition is false here sound/soc/tegra/tegra210_mbdrc.c 769 int tegra210_mbdrc_hw_params(struct snd_soc_component *cmpnt) 770 { 771 struct tegra210_ope *ope = snd_soc_component_get_drvdata(cmpnt); 772 const struct tegra210_mbdrc_config *conf = &mbdrc_init_config; 773 u32 val = 0; 774 unsigned int i; 775 776 regmap_read(ope->mbdrc_regmap, TEGRA210_MBDRC_CFG, &val); 777 --> 778 if (val & TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS is zero so this can't be true. #define TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS (0 << TEGRA210_MBDRC_CFG_MBDRC_MODE_SHIFT) The common false positive with this warning is that the macro is something which depends on the .config and in that case I just add it to the list in smatch_data/kernel.unconstant_macros. But in this case the macro is just always zero... Is there a plan to make it configurable or something? 779 return 0; 780 781 for (i = 0; i < MBDRC_NUM_BAND; i++) { 782 const struct tegra210_mbdrc_band_params *params = 783 &conf->band_params[i]; 784 785 u32 reg_off = i * TEGRA210_MBDRC_FILTER_PARAM_STRIDE; 786 787 tegra210_mbdrc_write_ram(ope->mbdrc_regmap, 788 reg_off + TEGRA210_MBDRC_CFG_RAM_CTRL, 789 reg_off + TEGRA210_MBDRC_CFG_RAM_DATA, 790 0, (u32 *)&params->biquad_params[0], 791 TEGRA210_MBDRC_MAX_BIQUAD_STAGES * 5); 792 } 793 return 0; 794 } regards, dan carpenter

2 3

pcm: Add error handler for `fgets`
by GitHub pull_request - opened 15 Jun '22

15 Jun '22

alsa-project/alsa-lib pull request #238 was opened from Korilakkuma: `fgets` function returns `NULL` if error occured. Therefore, end loop in the case. Request URL : https://github.com/alsa-project/alsa-lib/pull/238 Patch URL : https://github.com/alsa-project/alsa-lib/pull/238.patch Repository URL: https://github.com/alsa-project/alsa-lib

1 0

Any way to fuse static libasound with alsa-plugins so no dlopens are performed?
by GitHub issues - opened 14 Jun '22

14 Jun '22

alsa-project/alsa-plugins issue #45 was opened from MIvanchev: See title. I have a project that might benefit from such a mix. Issue URL : https://github.com/alsa-project/alsa-plugins/issues/45 Repository URL: https://github.com/alsa-project/alsa-plugins

1 0

[PATCH] ASoC: cs35l45: Add endianness flag in snd_soc_component_driver
by Charles Keepax 14 Jun '22

14 Jun '22

The endianness flag is used on the CODEC side to specify an ambivalence to endian, typically because it is lost over the hardware link. This device receives audio over an I2S DAI and as such should have endianness applied. Fixes: 0d463d016000 ("ASoC: cs35l45: Add driver for Cirrus Logic CS35L45 Smart Amp") Signed-off-by: Charles Keepax <ckeepax(a)opensource.cirrus.com> --- sound/soc/codecs/cs35l45.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/codecs/cs35l45.c b/sound/soc/codecs/cs35l45.c index c94edfce4b720..d15b3b77c7eb0 100644 --- a/sound/soc/codecs/cs35l45.c +++ b/sound/soc/codecs/cs35l45.c @@ -500,6 +500,8 @@ static const struct snd_soc_component_driver cs35l45_component = { .num_controls = ARRAY_SIZE(cs35l45_controls), .name = "cs35l45", + + .endianness = 1, }; static int __maybe_unused cs35l45_runtime_suspend(struct device *dev) -- 2.30.2

2 1

[PATCH] ASoC: ops: Fix boolean/integer detection for simple controls
by Mark Brown 14 Jun '22

14 Jun '22

The standard snd_soc_info_volsw() detects if a control is a volume control and needs to be reported as an integer even if it only has two values by looking for the string " Volume" in the control name. This results in false positives if the control has a name like "HP Volume Ramp Switch" since any " Volume" is matched, not just a trailing one. Fix this by making sure that we only match at the end of the control name. Signed-off-by: Mark Brown <broonie(a)kernel.org> --- sound/soc/soc-ops.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index e11109dcaa50..0267e39de2a8 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -176,13 +176,21 @@ int snd_soc_info_volsw(struct snd_kcontrol *kcontrol, { struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; + const char *vol_string = NULL; int max; max = uinfo->value.integer.max = mc->max - mc->min; if (mc->platform_max && mc->platform_max < max) max = mc->platform_max; - if (max == 1 && !strstr(kcontrol->id.name, " Volume")) + /* Even two value controls ending in Volume should always be integer */ + if (max == 1) { + vol_string = strstr(kcontrol->id.name, " Volume"); + if (vol_string && strcmp(vol_string, " Volume")) + vol_string = NULL; + } + + if (!vol_string) uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN; else uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER; -- 2.30.2

1 1

Re: [PATCH] ASoC: ti: omap-mcbsp: duplicate sysfs failure after PROBE_DEFER
by Mark Brown 14 Jun '22

14 Jun '22

On Tue, Jun 14, 2022 at 10:59:31AM -0500, David Owens wrote: > The call to sdma_pcm_platform_register() can return PROBE_DEFER, leading > to omap_mcbsp_init() being called multiple times. sysfs node creation > fails in subsequent calls to omap_mcbsp_init(), which prevents > the driver from ever successfully probing. The resulting errors can be > seen during boot: > > [ 1.749328] sysfs: cannot create duplicate filename '/devices/platform/68000000.ocp/49022000.mcbsp/max_tx_thres' > [ 1.759643] CPU: 0 PID: 6 Comm: kworker/u2:0 Not tainted 5.18.0-yocto-standard #1 > [ 1.767181] Hardware name: Generic OMAP36xx (Flattened Device Tree) > [ 1.773498] Workqueue: events_unbound deferred_probe_work_func > [ 1.779449] unwind_backtrace from show_stack+0x10/0x14 > [ 1.784729] show_stack from sysfs_warn_dup+0x4c/0x60 Please think hard before including complete backtraces in upstream reports, they are very large and contain almost no useful information relative to their size so often obscure the relevant content in your message. If part of the backtrace is usefully illustrative (it often is for search engines if nothing else) then it's usually better to pull out the relevant sections. > +++ b/sound/soc/ti/omap-mcbsp.c > @@ -1403,6 +1403,10 @@ static int asoc_mcbsp_probe(struct platform_device *pdev) > mcbsp->dev = &pdev->dev; > platform_set_drvdata(pdev, mcbsp); > > + ret = sdma_pcm_platform_register(&pdev->dev, "tx", "rx"); > + if (ret) > + return ret; > + > ret = omap_mcbsp_init(pdev); > if (ret) > return ret; > @@ -1412,13 +1416,9 @@ static int asoc_mcbsp_probe(struct platform_device *pdev) > omap_mcbsp_dai.capture.formats = SNDRV_PCM_FMTBIT_S16_LE; > } > > - ret = devm_snd_soc_register_component(&pdev->dev, > + return devm_snd_soc_register_component(&pdev->dev, > &omap_mcbsp_component, > &omap_mcbsp_dai, 1); > - if (ret) > - return ret; > - > - return sdma_pcm_platform_register(&pdev->dev, "tx", "rx"); > } It's not clear to me how this fixes the problem, your commit message doesn't mention how? I was expecting to see more error handling paths being added to unwind the sysfs allocation, or a conversion to devm. As things stand it's not clear to me that the error won't persist in the case where we defer registering the component.

1 0