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

RE: [PATCH 1/2] ASoC: sma1303: Add driver for Iron Device SMA1303 Amp
by Ki-Seok Jo 18 Aug '22

18 Aug '22

Dear Mark Brown, Thank you for your detailed feedback. I will apply what you told me as soon as possible and request it again. I'll ask you again if there's anything wrong with the correction. I appreciate your quick response. Thank you very much for your time Best Regards, Kiseok Jo Iron Device Corporation Tel.: +82-2-541-2896 Mobile: +82-10-3320-0376 Email: kiseok.jo(a)irondevice.com The information in this email and any attachment is confidential and may be legally protected. It is intended solely for the addressee. Access to this email by anyone else is unauthorised. If you are not the intended recipient, any disclosure or actions taken as a result of the information in this email is prohibited and may be unlawful. If you are not the intended recipient, please notify us immediately and then delete this e-mail and any attachment. Thank you. -----Original Message----- From: Mark Brown <broonie(a)kernel.org> Sent: Wednesday, August 17, 2022 9:09 PM To: Ki-Seok Jo <kiseok.jo(a)irondevice.com> Cc: Gyu-Hwa Park <gyuhwa.park(a)irondevice.com>; alsa-devel(a)alsa-project.org; Suk-Min Kang <sukmin.kang(a)irondevice.com> Subject: Re: [PATCH 1/2] ASoC: sma1303: Add driver for Iron Device SMA1303 Amp On Wed, Aug 17, 2022 at 12:29:37PM +0900, Kiseok Jo wrote: I got part way through reviewing this driver. The top level feedback here is that the driver appears to be exposing everything in the device to userspace as enumeration controls rather than using standard features like DAPM, or appropriate other control types where there's something that's a better fit than an enum. If there's a strong reason for the driver to do something so unusual then theere should be something in here which explains what's going on. > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* sma1303.c -- sma1303 ALSA SoC Audio driver > + * > + * Copyright 2019 Iron Device Corporation 2019? Please make the entire block a C++ one so things look more intentional. > +static int power_up_down_control_put(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) { > + struct snd_soc_component *component = > + snd_soc_kcontrol_component(kcontrol); > + struct sma1303_priv *sma1303 = snd_soc_component_get_drvdata(component); > + int sel = (int)ucontrol->value.integer.value[0]; > + > + if (sel && !(sma1303->force_amp_power_down)) > + sma1303_startup(component); > + else > + sma1303_shutdown(component); > + > + return 0; > +} This and all the other controls need to return 1 on change, please make sure a card with your driver your driver passes mixer-test cleanly - it will spot this and other problems. However I wouldn't expect the power controls to exist at all, generally power is managed via DAPM rather than userspace. > +static int force_sdo_bypass_put(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) { > + struct snd_soc_component *component = > + snd_soc_kcontrol_component(kcontrol); > + struct sma1303_priv *sma1303 = snd_soc_component_get_drvdata(component); > + int sel = (int)ucontrol->value.integer.value[0]; > + > + sma1303->sdo_bypass_flag = (bool)sel; > + > + return 0; > +} What is this bypassing? > +static const char * const sma1303_input_format_text[] = { > + "I2S", "LJ", "Reserved", "Reserved", > + "RJ_16", "RJ_18", "RJ_20", "RJ_24"}; > + > +static const struct soc_enum sma1303_input_format_enum = > +SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(sma1303_input_format_text), > + sma1303_input_format_text); This should be controlled by the machine driver through set_dai_fmt() and hw_params(). > +static const char * const sma1303_pcm_n_slot_text[] = { > + "Slot_1", "Slot_2", "Slot_3", "Slot_4", "Slot_5", "Slot_6", > + "Slot_7", "Slot_8", "Slot_9", "Slot_10", "Slot_11", "Slot_12", > + "Slot_13", "Slot_14", "Slot_15", "Slot_16"}; > + > +static const struct soc_enum sma1303_pcm_n_slot_enum = > +SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(sma1303_pcm_n_slot_text), > + sma1303_pcm_n_slot_text); set_tdm_slot() > +static const char * const sma1303_port_config_text[] = { > + "IN_Port", "Reserved", "OUT_Port", "Reserved"}; This should be DT properties, possibly using the pinctrl schema depending on what's actually going on. > +static const char * const sma1303_port_out_sel_text[] = { > + "Disable", "Format_C", "Mixer_out", "After_DSP", > + "Postscaler", "Reserved", "Reserved", "Reserved"}; > + > +static const struct soc_enum sma1303_port_out_sel_enum = > + SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(sma1303_port_out_sel_text), > + sma1303_port_out_sel_text); This looks like it should be a DAPM routing control. > +static const char * const sma1303_spk_off_slope_text[] = { > + "00", "01", "10", "11"}; > + > +static const struct soc_enum sma1303_spk_off_slope_enum = > + SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(sma1303_spk_off_slope_text), > + sma1303_spk_off_slope_text); Why make this an enum and not just a numerical control with values 0-3? > +static const char * const sma1303_spkmode_text[] = { > + "Off", "Mono", "Reserved", "Reserved", > + "Stereo", "Reserved", "Reserved", "Reserved"}; > + > +static const struct soc_enum sma1303_spkmode_enum = > + SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(sma1303_spkmode_text), > + sma1303_spkmode_text); Use a value enum to eliminate the reserved values from user selection, however if this is controlling if the device has mono or stereo speakers it should be a DT property since that's unlikely to change at runtime. > +static const char * const sma1303_input_gain_text[] = { > + "Gain_0dB", "Gain_M6dB", "Gain_M12dB", "Gain_MInf"}; > + > +static const struct soc_enum sma1303_input_gain_enum = > +SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(sma1303_input_gain_text), > + sma1303_input_gain_text); This should just be a normal volume control. I've stopped reviewing at this point.

1 0

snd_hda_intel: Woke up but no data
by GitHub issues - edited 18 Aug '22

18 Aug '22

alsa-project/alsa-lib issue #262 was edited from shmohawk: ``` Aug 17 01:11:23 hellokitty pulseaudio[12938]: [alsa-sink-CX20590 Analog] alsa-sink.c: ALSA woke us up to write new data to the device, but there was actually nothing to write. Aug 17 01:11:23 hellokitty pulseaudio[12938]: [alsa-sink-CX20590 Analog] alsa-sink.c: Most likely this is a bug in the ALSA driver 'snd_hda_intel'. Please report this issue to the ALSA developers. Aug 17 01:11:23 hellokitty pulseaudio[12938]: [alsa-sink-CX20590 Analog] alsa-sink.c: We were woken up with POLLOUT set -- however a subsequent snd_pcm_avail() returned 0 or another value < min_avail. ``` It is symptomous with a crackling sound for ~5 seconds after any playback starts. alsa-lib 1.2.7.2 alsa-utils 1.2.7 alsa-plugins 1.2.2 Issue URL : https://github.com/alsa-project/alsa-lib/issues/262 Repository URL: https://github.com/alsa-project/alsa-lib

1 0

snd_hda_intel: Woke up but no data
by GitHub issues - opened 18 Aug '22

18 Aug '22

alsa-project/alsa-lib issue #262 was opened from shmohawk: ``` Aug 17 01:11:23 hellokitty pulseaudio[12938]: [alsa-sink-CX20590 Analog] alsa-sink.c: ALSA woke us up to write new data to the device, but there was actually nothing to write. Aug 17 01:11:23 hellokitty pulseaudio[12938]: [alsa-sink-CX20590 Analog] alsa-sink.c: Most likely this is a bug in the ALSA driver 'snd_hda_intel'. Please report this issue to the ALSA developers. Aug 17 01:11:23 hellokitty pulseaudio[12938]: [alsa-sink-CX20590 Analog] alsa-sink.c: We were woken up with POLLOUT set -- however a subsequent snd_pcm_avail() returned 0 or another value < min_avail. ``` It is symptomous with a crackling sound for ~5 seconds after any playback starts. Issue URL : https://github.com/alsa-project/alsa-lib/issues/262 Repository URL: https://github.com/alsa-project/alsa-lib

1 0

[PATCH] ASoC: fsl_sai: Remove unnecessary FIFO reset in ISR
by Shengjiu Wang 17 Aug '22

17 Aug '22

The FIFO reset drops the words in the FIFO, which may cause channel swap when SAI module is running, especially when the DMA speed is low. So it is not good to do FIFO reset in ISR, then remove the operation. Fixes: e2681a1bf5ae ("ASoC: fsl_sai: Add isr to deal with error flag") Signed-off-by: Shengjiu Wang <shengjiu.wang(a)nxp.com> --- sound/soc/fsl/fsl_sai.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index d430eece1d6b..a7fa6f0bf83d 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -114,11 +114,8 @@ static irqreturn_t fsl_sai_isr(int irq, void *devid) if (flags & FSL_SAI_CSR_SEF) dev_dbg(dev, "isr: Tx Frame sync error detected\n"); - if (flags & FSL_SAI_CSR_FEF) { + if (flags & FSL_SAI_CSR_FEF) dev_dbg(dev, "isr: Transmit underrun detected\n"); - /* FIFO reset for safety */ - xcsr |= FSL_SAI_CSR_FR; - } if (flags & FSL_SAI_CSR_FWF) dev_dbg(dev, "isr: Enabled transmit FIFO is empty\n"); @@ -148,11 +145,8 @@ static irqreturn_t fsl_sai_isr(int irq, void *devid) if (flags & FSL_SAI_CSR_SEF) dev_dbg(dev, "isr: Rx Frame sync error detected\n"); - if (flags & FSL_SAI_CSR_FEF) { + if (flags & FSL_SAI_CSR_FEF) dev_dbg(dev, "isr: Receive overflow detected\n"); - /* FIFO reset for safety */ - xcsr |= FSL_SAI_CSR_FR; - } if (flags & FSL_SAI_CSR_FWF) dev_dbg(dev, "isr: Enabled receive FIFO is full\n"); -- 2.34.1

4 4

Re: [PATCH] ASoC: soc-utils-test: Add test for snd_soc_params_to_bclk()
by Mark Brown 17 Aug '22

17 Aug '22

On Wed, 17 Aug 2022 13:55:08 +0100, Richard Fitzgerald wrote: > snd_soc_params_to_bclk() calculates the BCLK from only the information in > snd_pcm_hw_params. It is therefore a subset of the functionality of > snd_soc_tdm_params_to_bclk() so can use a subset of the test case table. > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: soc-utils-test: Add test for snd_soc_params_to_bclk() commit: e32e23a2b588424aec0c4c4435530f8022318b8f All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark

1 0

[PATCH 0/5] ASoC: cs42l42: Some small code improvements
by Richard Fitzgerald 17 Aug '22

17 Aug '22

This is a collection of minor improvements to the code or comments. Richard Fitzgerald (5): ASoC: cs42l42: Don't include kernel.h ASoC: cs42l42: Add include dependencies to cs42l42.h ASoC: cs42l42: Move cs42l42_supply_names to .c file ASoC: cs42l42: Fix comment typo in cs42l42_slow_start_put() ASoC: cs42l42: Use snd_soc_tdm_params_to_bclk() sound/soc/codecs/cs42l42.c | 24 ++++++++++++++++-------- sound/soc/codecs/cs42l42.h | 13 +++++-------- 2 files changed, 21 insertions(+), 16 deletions(-) -- 2.30.2

2 3

[PATCH 0/4] ASoC/soundwire: log actual PING status on resume issues
by Bard Liao 17 Aug '22

17 Aug '22

we've been stuck with problems in the dual-amplifier configurations where one of the two devices seems to become UNATTACHED and never regains sync, see https://github.com/thesofproject/linux/issues/3638. This is a rather infrequent issue that may happen once or twice per month, but still it remains a concern. One possibility is that the device does lose sync but somehow our hardware detection fails to see it resync. This series just adds a basic read directly from the PING frames to help confirm if yes/no the device regain sync. The change is mainly on soundwire. @Mark, Could you ack the ASoC patch if it looks good to you? Pierre-Louis Bossart (4): soundwire: add read_ping_status helper definition in manager ops soundwire: intel/cadence: expose PING status in manager ops soundwire: add sdw_show_ping_status() helper ASoC: codecs: show PING status on resume failures drivers/soundwire/bus.c | 32 ++++++++++++++++++++++++++++++ drivers/soundwire/cadence_master.c | 8 ++++++++ drivers/soundwire/cadence_master.h | 2 ++ drivers/soundwire/intel.c | 1 + include/linux/soundwire/sdw.h | 5 +++++ sound/soc/codecs/max98373-sdw.c | 2 ++ sound/soc/codecs/rt1308-sdw.c | 2 ++ sound/soc/codecs/rt1316-sdw.c | 2 ++ sound/soc/codecs/rt5682-sdw.c | 2 ++ sound/soc/codecs/rt700-sdw.c | 2 ++ sound/soc/codecs/rt711-sdca-sdw.c | 2 ++ sound/soc/codecs/rt715-sdca-sdw.c | 2 ++ sound/soc/codecs/rt715-sdw.c | 2 ++ 13 files changed, 64 insertions(+) -- 2.25.1

4 10

[PATCH -next] ASoC: codecs: es8326: change es8326_regmap_config to static
by Yang Yingliang 17 Aug '22

17 Aug '22

es8326_regmap_config is only used in es8326.c now, change it to static. Signed-off-by: Yang Yingliang <yangyingliang(a)huawei.com> --- sound/soc/codecs/es8326.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/codecs/es8326.c b/sound/soc/codecs/es8326.c index 975302b2a61d..87c1cc16592b 100755 --- a/sound/soc/codecs/es8326.c +++ b/sound/soc/codecs/es8326.c @@ -207,7 +207,7 @@ static const struct regmap_access_table es8326_volatile_table = { .n_yes_ranges = ARRAY_SIZE(es8326_volatile_ranges), }; -const struct regmap_config es8326_regmap_config = { +static const struct regmap_config es8326_regmap_config = { .reg_bits = 8, .val_bits = 8, .max_register = 0xff, -- 2.25.1

2 1

[PATCH 0/4] Enable timestamp support for SOF compress driver
by Daniel Baluta 17 Aug '22

17 Aug '22

From: Daniel Baluta <daniel.baluta(a)nxp.com> This patchseries computes pcm_io_frames from the DAI position reported by SOF firmware. Using pcm_io_frames userspace applications can later compute timestamps for compressed stream. Daniel Baluta (2): ASoC: SOF: compress: Remove byte offset computation ASoC: SOF: compress: Introduce sof_compr_stream Laurentiu Mihalcea (2): ASoC: SOF: compress: Save channel count and sample bytes ASoC: SOF: compress: Add support for computing timestamps sound/soc/sof/compress.c | 54 ++++++++++++++++++++++++---------------- sound/soc/sof/sof-priv.h | 7 ++++++ 2 files changed, 40 insertions(+), 21 deletions(-) -- 2.27.0

2 5

[PATCH] ASoC: qcom: qdsp6: q6prm: add new clocks
by Srinivas Kandagatla 17 Aug '22

17 Aug '22

Add support to new clocks that are added in Q6DSP as part of newer version of LPASS support on SM8450 and SC8280XP. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla(a)linaro.org> --- .../sound/qcom,q6dsp-lpass-ports.h | 18 ++++++++++++++++++ sound/soc/qcom/qdsp6/q6prm-clocks.c | 9 +++++++++ sound/soc/qcom/qdsp6/q6prm.h | 19 +++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h index 0d3276c8fc11..9f7c5103bc82 100644 --- a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h +++ b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h @@ -193,6 +193,24 @@ #define LPASS_CLK_ID_RX_CORE_MCLK 59 #define LPASS_CLK_ID_RX_CORE_NPL_MCLK 60 #define LPASS_CLK_ID_VA_CORE_2X_MCLK 61 +/* Clock ID for MCLK for WSA2 core */ +#define LPASS_CLK_ID_WSA2_CORE_MCLK 62 +/* Clock ID for NPL MCLK for WSA2 core */ +#define LPASS_CLK_ID_WSA2_CORE_2X_MCLK 63 +/* Clock ID for RX Core TX MCLK */ +#define LPASS_CLK_ID_RX_CORE_TX_MCLK 64 +/* Clock ID for RX CORE TX 2X MCLK */ +#define LPASS_CLK_ID_RX_CORE_TX_2X_MCLK 65 +/* Clock ID for WSA core TX MCLK */ +#define LPASS_CLK_ID_WSA_CORE_TX_MCLK 66 +/* Clock ID for WSA core TX 2X MCLK */ +#define LPASS_CLK_ID_WSA_CORE_TX_2X_MCLK 67 +/* Clock ID for WSA2 core TX MCLK */ +#define LPASS_CLK_ID_WSA2_CORE_TX_MCLK 68 +/* Clock ID for WSA2 core TX 2X MCLK */ +#define LPASS_CLK_ID_WSA2_CORE_TX_2X_MCLK 69 +/* Clock ID for RX CORE MCLK2 2X MCLK */ +#define LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK 70 #define LPASS_HW_AVTIMER_VOTE 101 #define LPASS_HW_MACRO_VOTE 102 diff --git a/sound/soc/qcom/qdsp6/q6prm-clocks.c b/sound/soc/qcom/qdsp6/q6prm-clocks.c index a26cda5140c1..73b0cbac73d4 100644 --- a/sound/soc/qcom/qdsp6/q6prm-clocks.c +++ b/sound/soc/qcom/qdsp6/q6prm-clocks.c @@ -50,6 +50,15 @@ static const struct q6dsp_clk_init q6prm_clks[] = { Q6PRM_CLK(LPASS_CLK_ID_RX_CORE_MCLK), Q6PRM_CLK(LPASS_CLK_ID_RX_CORE_NPL_MCLK), Q6PRM_CLK(LPASS_CLK_ID_VA_CORE_2X_MCLK), + Q6PRM_CLK(LPASS_CLK_ID_WSA2_CORE_MCLK), + Q6PRM_CLK(LPASS_CLK_ID_WSA2_CORE_2X_MCLK), + Q6PRM_CLK(LPASS_CLK_ID_RX_CORE_TX_MCLK), + Q6PRM_CLK(LPASS_CLK_ID_RX_CORE_TX_2X_MCLK), + Q6PRM_CLK(LPASS_CLK_ID_WSA_CORE_TX_MCLK), + Q6PRM_CLK(LPASS_CLK_ID_WSA_CORE_TX_2X_MCLK), + Q6PRM_CLK(LPASS_CLK_ID_WSA2_CORE_TX_MCLK), + Q6PRM_CLK(LPASS_CLK_ID_WSA2_CORE_TX_2X_MCLK), + Q6PRM_CLK(LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK), Q6DSP_VOTE_CLK(LPASS_HW_MACRO_VOTE, Q6PRM_HW_CORE_ID_LPASS, "LPASS_HW_MACRO"), Q6DSP_VOTE_CLK(LPASS_HW_DCODEC_VOTE, Q6PRM_HW_CORE_ID_DCODEC, diff --git a/sound/soc/qcom/qdsp6/q6prm.h b/sound/soc/qcom/qdsp6/q6prm.h index fea4d1954bc1..a988a32086fe 100644 --- a/sound/soc/qcom/qdsp6/q6prm.h +++ b/sound/soc/qcom/qdsp6/q6prm.h @@ -64,6 +64,25 @@ #define Q6PRM_LPASS_CLK_ID_RX_CORE_MCLK 0x30e #define Q6PRM_LPASS_CLK_ID_RX_CORE_NPL_MCLK 0x30f +/* Clock ID for MCLK for WSA2 core */ +#define Q6PRM_LPASS_CLK_ID_WSA2_CORE_MCLK 0x310 +/* Clock ID for NPL MCLK for WSA2 core */ +#define Q6PRM_LPASS_CLK_ID_WSA2_CORE_2X_MCLK 0x311 +/* Clock ID for RX Core TX MCLK */ +#define Q6PRM_LPASS_CLK_ID_RX_CORE_TX_MCLK 0x312 +/* Clock ID for RX CORE TX 2X MCLK */ +#define Q6PRM_LPASS_CLK_ID_RX_CORE_TX_2X_MCLK 0x313 +/* Clock ID for WSA core TX MCLK */ +#define Q6PRM_LPASS_CLK_ID_WSA_CORE_TX_MCLK 0x314 +/* Clock ID for WSA core TX 2X MCLK */ +#define Q6PRM_LPASS_CLK_ID_WSA_CORE_TX_2X_MCLK 0x315 +/* Clock ID for WSA2 core TX MCLK */ +#define Q6PRM_LPASS_CLK_ID_WSA2_CORE_TX_MCLK 0x316 +/* Clock ID for WSA2 core TX 2X MCLK */ +#define Q6PRM_LPASS_CLK_ID_WSA2_CORE_TX_2X_MCLK 0x317 +/* Clock ID for RX CORE MCLK2 2X MCLK */ +#define Q6PRM_LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK 0x318 + #define Q6PRM_LPASS_CLK_SRC_INTERNAL 1 #define Q6PRM_LPASS_CLK_ROOT_DEFAULT 0 #define Q6PRM_HW_CORE_ID_LPASS 1 -- 2.21.0

2 1