2022-04-07 14:25:36

by Srinivas Kandagatla

[permalink] [raw]
Subject: [PATCH] ASoC: codecs: wcd934x: do not switch off SIDO Buck when codec is in use

SIDO(Single-Inductor Dual-Ouput) Buck powers up both analog and digital
circuits along with internal memory, powering off this is the last thing
that codec should do when going to very low power.

Current code was powering off this Buck if there are no users of sysclk,
which is not correct. Powering off this buck will result in no register access.
This code path was never tested until recently after adding pm support
in SoundWire controller. Fix this by removing the buck poweroff when the
codec is active and also the code that is not used.

Without this patch all the read/write transactions will never complete and
results in SLIMBus Errors like:

qcom,slim-ngd qcom,slim-ngd.1: Tx:MT:0x0, MC:0x60, LA:0xcf failed:-110
wcd934x-codec wcd934x-codec.1.auto: ASoC: error at soc_component_read_no_lock
on wcd934x-codec.1.auto for register: [0x00000d05] -110
qcom,slim-ngd-ctrl 171c0000.slim: Error Interrupt received 0x82000000

Reported-by: Amit Pundir <[email protected]>
Fixes: a61f3b4f476e ("ASoC: wcd934x: add support to wcd9340/wcd9341 codec")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
sound/soc/codecs/wcd934x.c | 26 +-------------------------
1 file changed, 1 insertion(+), 25 deletions(-)

diff --git a/sound/soc/codecs/wcd934x.c b/sound/soc/codecs/wcd934x.c
index 1e75e93cf28f..6298ebe96e94 100644
--- a/sound/soc/codecs/wcd934x.c
+++ b/sound/soc/codecs/wcd934x.c
@@ -1274,29 +1274,7 @@ static int wcd934x_set_sido_input_src(struct wcd934x_codec *wcd, int sido_src)
if (sido_src == wcd->sido_input_src)
return 0;

- if (sido_src == SIDO_SOURCE_INTERNAL) {
- regmap_update_bits(wcd->regmap, WCD934X_ANA_BUCK_CTL,
- WCD934X_ANA_BUCK_HI_ACCU_EN_MASK, 0);
- usleep_range(100, 110);
- regmap_update_bits(wcd->regmap, WCD934X_ANA_BUCK_CTL,
- WCD934X_ANA_BUCK_HI_ACCU_PRE_ENX_MASK, 0x0);
- usleep_range(100, 110);
- regmap_update_bits(wcd->regmap, WCD934X_ANA_RCO,
- WCD934X_ANA_RCO_BG_EN_MASK, 0);
- usleep_range(100, 110);
- regmap_update_bits(wcd->regmap, WCD934X_ANA_BUCK_CTL,
- WCD934X_ANA_BUCK_PRE_EN1_MASK,
- WCD934X_ANA_BUCK_PRE_EN1_ENABLE);
- usleep_range(100, 110);
- regmap_update_bits(wcd->regmap, WCD934X_ANA_BUCK_CTL,
- WCD934X_ANA_BUCK_PRE_EN2_MASK,
- WCD934X_ANA_BUCK_PRE_EN2_ENABLE);
- usleep_range(100, 110);
- regmap_update_bits(wcd->regmap, WCD934X_ANA_BUCK_CTL,
- WCD934X_ANA_BUCK_HI_ACCU_EN_MASK,
- WCD934X_ANA_BUCK_HI_ACCU_ENABLE);
- usleep_range(100, 110);
- } else if (sido_src == SIDO_SOURCE_RCO_BG) {
+ if (sido_src == SIDO_SOURCE_RCO_BG) {
regmap_update_bits(wcd->regmap, WCD934X_ANA_RCO,
WCD934X_ANA_RCO_BG_EN_MASK,
WCD934X_ANA_RCO_BG_ENABLE);
@@ -1382,8 +1360,6 @@ static int wcd934x_disable_ana_bias_and_syclk(struct wcd934x_codec *wcd)
regmap_update_bits(wcd->regmap, WCD934X_CLK_SYS_MCLK_PRG,
WCD934X_EXT_CLK_BUF_EN_MASK |
WCD934X_MCLK_EN_MASK, 0x0);
- wcd934x_set_sido_input_src(wcd, SIDO_SOURCE_INTERNAL);
-
regmap_update_bits(wcd->regmap, WCD934X_ANA_BIAS,
WCD934X_ANA_BIAS_EN_MASK, 0);
regmap_update_bits(wcd->regmap, WCD934X_ANA_BIAS,
--
2.21.0


2022-04-07 20:48:10

by Amit Pundir

[permalink] [raw]
Subject: Re: [PATCH] ASoC: codecs: wcd934x: do not switch off SIDO Buck when codec is in use

On Thu, 7 Apr 2022 at 15:13, Srinivas Kandagatla
<[email protected]> wrote:
>
> SIDO(Single-Inductor Dual-Ouput) Buck powers up both analog and digital
> circuits along with internal memory, powering off this is the last thing
> that codec should do when going to very low power.
>
> Current code was powering off this Buck if there are no users of sysclk,
> which is not correct. Powering off this buck will result in no register access.
> This code path was never tested until recently after adding pm support
> in SoundWire controller. Fix this by removing the buck poweroff when the
> codec is active and also the code that is not used.
>
> Without this patch all the read/write transactions will never complete and
> results in SLIMBus Errors like:
>
> qcom,slim-ngd qcom,slim-ngd.1: Tx:MT:0x0, MC:0x60, LA:0xcf failed:-110
> wcd934x-codec wcd934x-codec.1.auto: ASoC: error at soc_component_read_no_lock
> on wcd934x-codec.1.auto for register: [0x00000d05] -110
> qcom,slim-ngd-ctrl 171c0000.slim: Error Interrupt received 0x82000000
>

Thanks Srinivas. It fixes the above regression I see on DB845c and
Xiaomi Pocophone F1 running AOSP with v5.18-rc1.

Tested-by: Amit Pundir <[email protected]>

> Reported-by: Amit Pundir <[email protected]>
> Fixes: a61f3b4f476e ("ASoC: wcd934x: add support to wcd9340/wcd9341 codec")
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> sound/soc/codecs/wcd934x.c | 26 +-------------------------
> 1 file changed, 1 insertion(+), 25 deletions(-)
>
> diff --git a/sound/soc/codecs/wcd934x.c b/sound/soc/codecs/wcd934x.c
> index 1e75e93cf28f..6298ebe96e94 100644
> --- a/sound/soc/codecs/wcd934x.c
> +++ b/sound/soc/codecs/wcd934x.c
> @@ -1274,29 +1274,7 @@ static int wcd934x_set_sido_input_src(struct wcd934x_codec *wcd, int sido_src)
> if (sido_src == wcd->sido_input_src)
> return 0;
>
> - if (sido_src == SIDO_SOURCE_INTERNAL) {
> - regmap_update_bits(wcd->regmap, WCD934X_ANA_BUCK_CTL,
> - WCD934X_ANA_BUCK_HI_ACCU_EN_MASK, 0);
> - usleep_range(100, 110);
> - regmap_update_bits(wcd->regmap, WCD934X_ANA_BUCK_CTL,
> - WCD934X_ANA_BUCK_HI_ACCU_PRE_ENX_MASK, 0x0);
> - usleep_range(100, 110);
> - regmap_update_bits(wcd->regmap, WCD934X_ANA_RCO,
> - WCD934X_ANA_RCO_BG_EN_MASK, 0);
> - usleep_range(100, 110);
> - regmap_update_bits(wcd->regmap, WCD934X_ANA_BUCK_CTL,
> - WCD934X_ANA_BUCK_PRE_EN1_MASK,
> - WCD934X_ANA_BUCK_PRE_EN1_ENABLE);
> - usleep_range(100, 110);
> - regmap_update_bits(wcd->regmap, WCD934X_ANA_BUCK_CTL,
> - WCD934X_ANA_BUCK_PRE_EN2_MASK,
> - WCD934X_ANA_BUCK_PRE_EN2_ENABLE);
> - usleep_range(100, 110);
> - regmap_update_bits(wcd->regmap, WCD934X_ANA_BUCK_CTL,
> - WCD934X_ANA_BUCK_HI_ACCU_EN_MASK,
> - WCD934X_ANA_BUCK_HI_ACCU_ENABLE);
> - usleep_range(100, 110);
> - } else if (sido_src == SIDO_SOURCE_RCO_BG) {
> + if (sido_src == SIDO_SOURCE_RCO_BG) {
> regmap_update_bits(wcd->regmap, WCD934X_ANA_RCO,
> WCD934X_ANA_RCO_BG_EN_MASK,
> WCD934X_ANA_RCO_BG_ENABLE);
> @@ -1382,8 +1360,6 @@ static int wcd934x_disable_ana_bias_and_syclk(struct wcd934x_codec *wcd)
> regmap_update_bits(wcd->regmap, WCD934X_CLK_SYS_MCLK_PRG,
> WCD934X_EXT_CLK_BUF_EN_MASK |
> WCD934X_MCLK_EN_MASK, 0x0);
> - wcd934x_set_sido_input_src(wcd, SIDO_SOURCE_INTERNAL);
> -
> regmap_update_bits(wcd->regmap, WCD934X_ANA_BIAS,
> WCD934X_ANA_BIAS_EN_MASK, 0);
> regmap_update_bits(wcd->regmap, WCD934X_ANA_BIAS,
> --
> 2.21.0
>

2022-04-08 15:01:12

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] ASoC: codecs: wcd934x: do not switch off SIDO Buck when codec is in use

On Thu, 7 Apr 2022 10:43:13 +0100, Srinivas Kandagatla wrote:
> SIDO(Single-Inductor Dual-Ouput) Buck powers up both analog and digital
> circuits along with internal memory, powering off this is the last thing
> that codec should do when going to very low power.
>
> Current code was powering off this Buck if there are no users of sysclk,
> which is not correct. Powering off this buck will result in no register access.
> This code path was never tested until recently after adding pm support
> in SoundWire controller. Fix this by removing the buck poweroff when the
> codec is active and also the code that is not used.
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: codecs: wcd934x: do not switch off SIDO Buck when codec is in use
commit: db6dd1bee63d1d88fbddfe07af800af5948ac28e

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