[PATCH 5.10.y-cip 0/2] ASoC: Renesas: Backport audio fixes

[PATCH 5.10.y-cip 0/2] ASoC: Renesas: Backport audio fixes

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Hi,

Series backports audio fixes necessary for the Renesas RZ/G3S SoC
and the Renesas RZ SMARC Carrier-II board.

Thank you,
Claudiu

Claudiu Beznea (2):
  ASoC: da7213: Use component driver suspend/resume
  ASoC: renesas: rz-ssi: Use proper dma_buffer_pos after resume

 sound/soc/codecs/da7213.c | 65 ++++++++++++++++++++++++---------------
 sound/soc/codecs/da7213.h |  1 +
 sound/soc/sh/rz-ssi.c     | 25 ++++++++-------
 3 files changed, 54 insertions(+), 37 deletions(-)

-- 
2.43.0

[PATCH 5.10.y-cip 1/2] ASoC: da7213: Use component driver suspend/resume

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

commit 249d96b492efb7a773296ab2c62179918301c146 upstream.

Since snd_soc_suspend() is invoked through snd_soc_pm_ops->suspend(),
and snd_soc_pm_ops is associated with the soc_driver (defined in
sound/soc/soc-core.c), and there is no parent-child relationship between
the soc_driver and the DA7213 codec driver, the power management subsystem
does not enforce a specific suspend/resume order between the DA7213 driver
and the soc_driver.

Because of this, the different codec component functionalities, called from
snd_soc_resume() to reconfigure various functions, can race with the
DA7213 struct dev_pm_ops::resume function, leading to misapplied
configuration. This occasionally results in clipped sound.

Fix this by dropping the struct dev_pm_ops::{suspend, resume} and use
instead struct snd_soc_component_driver::{suspend, resume}. This ensures
the proper configuration sequence is handled by the ASoC subsystem.

Cc: stable@vger.kernel.org
Fixes: 431e040065c8 ("ASoC: da7213: Add suspend to RAM support")
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Link: https://patch.msgid.link/20251104114914.2060603-1-claudiu.beznea.uj@bp.renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
[claudiu.beznea: fixed conflict by keeping the code from this patch and
 using SET_RUNTIME_PM_OPS() instead of RUNTIME_PM_OPS()]
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---
 sound/soc/codecs/da7213.c | 65 ++++++++++++++++++++++++---------------
 sound/soc/codecs/da7213.h |  1 +
 2 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
index 6d32026ae69f..b4f4f4ebafaf 100644
--- a/sound/soc/codecs/da7213.c
+++ b/sound/soc/codecs/da7213.c
@@ -2106,11 +2106,50 @@ static int da7213_probe(struct snd_soc_component *component)
 	return 0;
 }
 
+static int da7213_runtime_suspend(struct device *dev)
+{
+	struct da7213_priv *da7213 = dev_get_drvdata(dev);
+
+	regcache_cache_only(da7213->regmap, true);
+	regcache_mark_dirty(da7213->regmap);
+	regulator_bulk_disable(DA7213_NUM_SUPPLIES, da7213->supplies);
+
+	return 0;
+}
+
+static int da7213_runtime_resume(struct device *dev)
+{
+	struct da7213_priv *da7213 = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regulator_bulk_enable(DA7213_NUM_SUPPLIES, da7213->supplies);
+	if (ret < 0)
+		return ret;
+	regcache_cache_only(da7213->regmap, false);
+	return regcache_sync(da7213->regmap);
+}
+
+static int da7213_suspend(struct snd_soc_component *component)
+{
+	struct da7213_priv *da7213 = snd_soc_component_get_drvdata(component);
+
+	return da7213_runtime_suspend(da7213->dev);
+}
+
+static int da7213_resume(struct snd_soc_component *component)
+{
+	struct da7213_priv *da7213 = snd_soc_component_get_drvdata(component);
+
+	return da7213_runtime_resume(da7213->dev);
+}
+
 static const struct snd_soc_component_driver soc_component_dev_da7213 = {
 	.probe			= da7213_probe,
 	.set_bias_level		= da7213_set_bias_level,
 	.controls		= da7213_snd_controls,
 	.num_controls		= ARRAY_SIZE(da7213_snd_controls),
+	.suspend		= da7213_suspend,
+	.resume			= da7213_resume,
 	.dapm_widgets		= da7213_dapm_widgets,
 	.num_dapm_widgets	= ARRAY_SIZE(da7213_dapm_widgets),
 	.dapm_routes		= da7213_audio_map,
@@ -2159,6 +2198,8 @@ static int da7213_i2c_probe(struct i2c_client *i2c,
 	if (!da7213->fin_min_rate)
 		return -EINVAL;
 
+	da7213->dev = &i2c->dev;
+
 	i2c_set_clientdata(i2c, da7213);
 
 	/* Get required supplies */
@@ -2210,32 +2251,8 @@ static int da7213_i2c_remove(struct i2c_client *i2c)
 	return 0;
 }
 
-static int __maybe_unused da7213_runtime_suspend(struct device *dev)
-{
-	struct da7213_priv *da7213 = dev_get_drvdata(dev);
-
-	regcache_cache_only(da7213->regmap, true);
-	regcache_mark_dirty(da7213->regmap);
-	regulator_bulk_disable(DA7213_NUM_SUPPLIES, da7213->supplies);
-
-	return 0;
-}
-
-static int __maybe_unused da7213_runtime_resume(struct device *dev)
-{
-	struct da7213_priv *da7213 = dev_get_drvdata(dev);
-	int ret;
-
-	ret = regulator_bulk_enable(DA7213_NUM_SUPPLIES, da7213->supplies);
-	if (ret < 0)
-		return ret;
-	regcache_cache_only(da7213->regmap, false);
-	return regcache_sync(da7213->regmap);
-}
-
 static const struct dev_pm_ops da7213_pm = {
 	SET_RUNTIME_PM_OPS(da7213_runtime_suspend, da7213_runtime_resume, NULL)
-	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
 };
 
 static const struct i2c_device_id da7213_i2c_id[] = {
diff --git a/sound/soc/codecs/da7213.h b/sound/soc/codecs/da7213.h
index b9ab791d6b88..29cbf0eb6124 100644
--- a/sound/soc/codecs/da7213.h
+++ b/sound/soc/codecs/da7213.h
@@ -595,6 +595,7 @@ enum da7213_supplies {
 /* Codec private data */
 struct da7213_priv {
 	struct regmap *regmap;
+	struct device *dev;
 	struct mutex ctrl_lock;
 	struct regulator_bulk_data supplies[DA7213_NUM_SUPPLIES];
 	struct clk *mclk;
-- 
2.43.0

Re: [cip-dev] [PATCH 5.10.y-cip 1/2] ASoC: da7213: Use component driver suspend/resume

[Nobuhiro Iwamatsu (Toshiba)]

Hi,

2025年11月22日(土) 19:51 claudiu beznea via lists.cip-project.org
<claudiu.beznea=tuxon.dev@lists.cip-project.org>:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> commit 249d96b492efb7a773296ab2c62179918301c146 upstream.
>
> Since snd_soc_suspend() is invoked through snd_soc_pm_ops->suspend(),
> and snd_soc_pm_ops is associated with the soc_driver (defined in
> sound/soc/soc-core.c), and there is no parent-child relationship between
> the soc_driver and the DA7213 codec driver, the power management subsystem
> does not enforce a specific suspend/resume order between the DA7213 driver
> and the soc_driver.
>
> Because of this, the different codec component functionalities, called from
> snd_soc_resume() to reconfigure various functions, can race with the
> DA7213 struct dev_pm_ops::resume function, leading to misapplied
> configuration. This occasionally results in clipped sound.
>
> Fix this by dropping the struct dev_pm_ops::{suspend, resume} and use
> instead struct snd_soc_component_driver::{suspend, resume}. This ensures
> the proper configuration sequence is handled by the ASoC subsystem.
>
> Cc: stable@vger.kernel.org
> Fixes: 431e040065c8 ("ASoC: da7213: Add suspend to RAM support")
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Link: https://patch.msgid.link/20251104114914.2060603-1-claudiu.beznea.uj@bp.renesas.com
> Signed-off-by: Mark Brown <broonie@kernel.org>
> [claudiu.beznea: fixed conflict by keeping the code from this patch and
>  using SET_RUNTIME_PM_OPS() instead of RUNTIME_PM_OPS()]
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>  sound/soc/codecs/da7213.c | 65 ++++++++++++++++++++++++---------------
>  sound/soc/codecs/da7213.h |  1 +
>  2 files changed, 42 insertions(+), 24 deletions(-)
>
> diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
> index 6d32026ae69f..b4f4f4ebafaf 100644
> --- a/sound/soc/codecs/da7213.c
> +++ b/sound/soc/codecs/da7213.c
> @@ -2106,11 +2106,50 @@ static int da7213_probe(struct snd_soc_component *component)
>         return 0;
>  }
>
> +static int da7213_runtime_suspend(struct device *dev)
> +{
> +       struct da7213_priv *da7213 = dev_get_drvdata(dev);
> +
> +       regcache_cache_only(da7213->regmap, true);
> +       regcache_mark_dirty(da7213->regmap);
> +       regulator_bulk_disable(DA7213_NUM_SUPPLIES, da7213->supplies);

There is no error checking.
We need to add error checking or return an error code.

> +
> +       return 0;
> +}
> +
> +static int da7213_runtime_resume(struct device *dev)
> +{
> +       struct da7213_priv *da7213 = dev_get_drvdata(dev);
> +       int ret;
> +
> +       ret = regulator_bulk_enable(DA7213_NUM_SUPPLIES, da7213->supplies);
> +       if (ret < 0)
> +               return ret;
> +       regcache_cache_only(da7213->regmap, false);
> +       return regcache_sync(da7213->regmap);
> +}
> +
> +static int da7213_suspend(struct snd_soc_component *component)
> +{
> +       struct da7213_priv *da7213 = snd_soc_component_get_drvdata(component);
> +
> +       return da7213_runtime_suspend(da7213->dev);
> +}
> +
> +static int da7213_resume(struct snd_soc_component *component)
> +{
> +       struct da7213_priv *da7213 = snd_soc_component_get_drvdata(component);
> +
> +       return da7213_runtime_resume(da7213->dev);
> +}
> +
>  static const struct snd_soc_component_driver soc_component_dev_da7213 = {
>         .probe                  = da7213_probe,
>         .set_bias_level         = da7213_set_bias_level,
>         .controls               = da7213_snd_controls,
>         .num_controls           = ARRAY_SIZE(da7213_snd_controls),
> +       .suspend                = da7213_suspend,
> +       .resume                 = da7213_resume,
>         .dapm_widgets           = da7213_dapm_widgets,
>         .num_dapm_widgets       = ARRAY_SIZE(da7213_dapm_widgets),
>         .dapm_routes            = da7213_audio_map,
> @@ -2159,6 +2198,8 @@ static int da7213_i2c_probe(struct i2c_client *i2c,
>         if (!da7213->fin_min_rate)
>                 return -EINVAL;
>
> +       da7213->dev = &i2c->dev;
> +
>         i2c_set_clientdata(i2c, da7213);
>
>         /* Get required supplies */
> @@ -2210,32 +2251,8 @@ static int da7213_i2c_remove(struct i2c_client *i2c)
>         return 0;
>  }
>
> -static int __maybe_unused da7213_runtime_suspend(struct device *dev)
> -{
> -       struct da7213_priv *da7213 = dev_get_drvdata(dev);
> -
> -       regcache_cache_only(da7213->regmap, true);
> -       regcache_mark_dirty(da7213->regmap);
> -       regulator_bulk_disable(DA7213_NUM_SUPPLIES, da7213->supplies);
> -
> -       return 0;
> -}
> -
> -static int __maybe_unused da7213_runtime_resume(struct device *dev)
> -{
> -       struct da7213_priv *da7213 = dev_get_drvdata(dev);
> -       int ret;
> -
> -       ret = regulator_bulk_enable(DA7213_NUM_SUPPLIES, da7213->supplies);
> -       if (ret < 0)
> -               return ret;
> -       regcache_cache_only(da7213->regmap, false);
> -       return regcache_sync(da7213->regmap);
> -}
> -
>  static const struct dev_pm_ops da7213_pm = {
>         SET_RUNTIME_PM_OPS(da7213_runtime_suspend, da7213_runtime_resume, NULL)
> -       SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
>  };
>
>  static const struct i2c_device_id da7213_i2c_id[] = {
> diff --git a/sound/soc/codecs/da7213.h b/sound/soc/codecs/da7213.h
> index b9ab791d6b88..29cbf0eb6124 100644
> --- a/sound/soc/codecs/da7213.h
> +++ b/sound/soc/codecs/da7213.h
> @@ -595,6 +595,7 @@ enum da7213_supplies {
>  /* Codec private data */
>  struct da7213_priv {
>         struct regmap *regmap;
> +       struct device *dev;
>         struct mutex ctrl_lock;
>         struct regulator_bulk_data supplies[DA7213_NUM_SUPPLIES];
>         struct clk *mclk;
> --
> 2.43.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#20688): https://lists.cip-project.org/g/cip-dev/message/20688
> Mute This Topic: https://lists.cip-project.org/mt/116421641/4520494
> Group Owner: cip-dev+owner@lists.cip-project.org
> Unsubscribe: https://lists.cip-project.org/g/cip-dev/unsub [iwamatsu@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Best regards,
  Nobuhiro
-- 
Nobuhiro Iwamatsu

Re: [cip-dev] [PATCH 5.10.y-cip 1/2] ASoC: da7213: Use component driver suspend/resume

[Claudiu Beznea]

Hi, Nobuhiro,

On 11/24/25 02:16, Nobuhiro Iwamatsu (Toshiba) wrote:
> Hi,
> 
> 2025年11月22日(土) 19:51 claudiu beznea via lists.cip-project.org
> <claudiu.beznea=tuxon.dev@lists.cip-project.org>:
>>
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> commit 249d96b492efb7a773296ab2c62179918301c146 upstream.
>>
>> Since snd_soc_suspend() is invoked through snd_soc_pm_ops->suspend(),
>> and snd_soc_pm_ops is associated with the soc_driver (defined in
>> sound/soc/soc-core.c), and there is no parent-child relationship between
>> the soc_driver and the DA7213 codec driver, the power management subsystem
>> does not enforce a specific suspend/resume order between the DA7213 driver
>> and the soc_driver.
>>
>> Because of this, the different codec component functionalities, called from
>> snd_soc_resume() to reconfigure various functions, can race with the
>> DA7213 struct dev_pm_ops::resume function, leading to misapplied
>> configuration. This occasionally results in clipped sound.
>>
>> Fix this by dropping the struct dev_pm_ops::{suspend, resume} and use
>> instead struct snd_soc_component_driver::{suspend, resume}. This ensures
>> the proper configuration sequence is handled by the ASoC subsystem.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 431e040065c8 ("ASoC: da7213: Add suspend to RAM support")
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> Link: https://patch.msgid.link/20251104114914.2060603-1-claudiu.beznea.uj@bp.renesas.com
>> Signed-off-by: Mark Brown <broonie@kernel.org>
>> [claudiu.beznea: fixed conflict by keeping the code from this patch and
>>  using SET_RUNTIME_PM_OPS() instead of RUNTIME_PM_OPS()]
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>  sound/soc/codecs/da7213.c | 65 ++++++++++++++++++++++++---------------
>>  sound/soc/codecs/da7213.h |  1 +
>>  2 files changed, 42 insertions(+), 24 deletions(-)
>>
>> diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
>> index 6d32026ae69f..b4f4f4ebafaf 100644
>> --- a/sound/soc/codecs/da7213.c
>> +++ b/sound/soc/codecs/da7213.c
>> @@ -2106,11 +2106,50 @@ static int da7213_probe(struct snd_soc_component *component)
>>         return 0;
>>  }
>>
>> +static int da7213_runtime_suspend(struct device *dev)
>> +{
>> +       struct da7213_priv *da7213 = dev_get_drvdata(dev);
>> +
>> +       regcache_cache_only(da7213->regmap, true);
>> +       regcache_mark_dirty(da7213->regmap);
>> +       regulator_bulk_disable(DA7213_NUM_SUPPLIES, da7213->supplies);
> 
> There is no error checking.
> We need to add error checking or return an error code.

This is just a move of da7213_runtime_suspend() to be able to use it in
da7213_suspend() w/o forward declaration.

Do you want me to adjust it for CIP?

Thank you,
Claudiu

> 
>> +
>> +       return 0;
>> +}
>> +
>> +static int da7213_runtime_resume(struct device *dev)
>> +{
>> +       struct da7213_priv *da7213 = dev_get_drvdata(dev);
>> +       int ret;
>> +
>> +       ret = regulator_bulk_enable(DA7213_NUM_SUPPLIES, da7213->supplies);
>> +       if (ret < 0)
>> +               return ret;
>> +       regcache_cache_only(da7213->regmap, false);
>> +       return regcache_sync(da7213->regmap);
>> +}
>> +
>> +static int da7213_suspend(struct snd_soc_component *component)
>> +{
>> +       struct da7213_priv *da7213 = snd_soc_component_get_drvdata(component);
>> +
>> +       return da7213_runtime_suspend(da7213->dev);
>> +}
>> +
>> +static int da7213_resume(struct snd_soc_component *component)
>> +{
>> +       struct da7213_priv *da7213 = snd_soc_component_get_drvdata(component);
>> +
>> +       return da7213_runtime_resume(da7213->dev);
>> +}
>> +
>>  static const struct snd_soc_component_driver soc_component_dev_da7213 = {
>>         .probe                  = da7213_probe,
>>         .set_bias_level         = da7213_set_bias_level,
>>         .controls               = da7213_snd_controls,
>>         .num_controls           = ARRAY_SIZE(da7213_snd_controls),
>> +       .suspend                = da7213_suspend,
>> +       .resume                 = da7213_resume,
>>         .dapm_widgets           = da7213_dapm_widgets,
>>         .num_dapm_widgets       = ARRAY_SIZE(da7213_dapm_widgets),
>>         .dapm_routes            = da7213_audio_map,
>> @@ -2159,6 +2198,8 @@ static int da7213_i2c_probe(struct i2c_client *i2c,
>>         if (!da7213->fin_min_rate)
>>                 return -EINVAL;
>>
>> +       da7213->dev = &i2c->dev;
>> +
>>         i2c_set_clientdata(i2c, da7213);
>>
>>         /* Get required supplies */
>> @@ -2210,32 +2251,8 @@ static int da7213_i2c_remove(struct i2c_client *i2c)
>>         return 0;
>>  }
>>
>> -static int __maybe_unused da7213_runtime_suspend(struct device *dev)
>> -{
>> -       struct da7213_priv *da7213 = dev_get_drvdata(dev);
>> -
>> -       regcache_cache_only(da7213->regmap, true);
>> -       regcache_mark_dirty(da7213->regmap);
>> -       regulator_bulk_disable(DA7213_NUM_SUPPLIES, da7213->supplies);
>> -
>> -       return 0;
>> -}
>> -
>> -static int __maybe_unused da7213_runtime_resume(struct device *dev)
>> -{
>> -       struct da7213_priv *da7213 = dev_get_drvdata(dev);
>> -       int ret;
>> -
>> -       ret = regulator_bulk_enable(DA7213_NUM_SUPPLIES, da7213->supplies);
>> -       if (ret < 0)
>> -               return ret;
>> -       regcache_cache_only(da7213->regmap, false);
>> -       return regcache_sync(da7213->regmap);
>> -}
>> -
>>  static const struct dev_pm_ops da7213_pm = {
>>         SET_RUNTIME_PM_OPS(da7213_runtime_suspend, da7213_runtime_resume, NULL)
>> -       SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
>>  };
>>
>>  static const struct i2c_device_id da7213_i2c_id[] = {
>> diff --git a/sound/soc/codecs/da7213.h b/sound/soc/codecs/da7213.h
>> index b9ab791d6b88..29cbf0eb6124 100644
>> --- a/sound/soc/codecs/da7213.h
>> +++ b/sound/soc/codecs/da7213.h
>> @@ -595,6 +595,7 @@ enum da7213_supplies {
>>  /* Codec private data */
>>  struct da7213_priv {
>>         struct regmap *regmap;
>> +       struct device *dev;
>>         struct mutex ctrl_lock;
>>         struct regulator_bulk_data supplies[DA7213_NUM_SUPPLIES];
>>         struct clk *mclk;
>> --
>> 2.43.0
>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#20688): https://lists.cip-project.org/g/cip-dev/message/20688
>> Mute This Topic: https://lists.cip-project.org/mt/116421641/4520494
>> Group Owner: cip-dev+owner@lists.cip-project.org
>> Unsubscribe: https://lists.cip-project.org/g/cip-dev/unsub [iwamatsu@gmail.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
> 
> Best regards,
>   Nobuhiro

Re: [cip-dev] [PATCH 5.10.y-cip 1/2] ASoC: da7213: Use component driver suspend/resume

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1131 bytes --]

Hi!

> >> +++ b/sound/soc/codecs/da7213.c
> >> @@ -2106,11 +2106,50 @@ static int da7213_probe(struct snd_soc_component *component)
> >>         return 0;
> >>  }
> >>
> >> +static int da7213_runtime_suspend(struct device *dev)
> >> +{
> >> +       struct da7213_priv *da7213 = dev_get_drvdata(dev);
> >> +
> >> +       regcache_cache_only(da7213->regmap, true);
> >> +       regcache_mark_dirty(da7213->regmap);
> >> +       regulator_bulk_disable(DA7213_NUM_SUPPLIES, da7213->supplies);
> > 
> > There is no error checking.
> > We need to add error checking or return an error code.
> 
> This is just a move of da7213_runtime_suspend() to be able to use it in
> da7213_suspend() w/o forward declaration.
> 
> Do you want me to adjust it for CIP?

I don't believe this should block merge, but you may want to
investigate error handling here and eventually fix it up in the
mainline. At that point it may be worth backporting to -cip (or not).

Best regards,
								Pavel
-- 
In cooperation with DENX Software Engineering GmbH, HRB 165235 Munich,
Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [cip-dev] [PATCH 5.10.y-cip 1/2] ASoC: da7213: Use component driver suspend/resume

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]

Hi!

> >> +static int da7213_runtime_suspend(struct device *dev)
> >> +{
> >> +       struct da7213_priv *da7213 = dev_get_drvdata(dev);
> >> +
> >> +       regcache_cache_only(da7213->regmap, true);
> >> +       regcache_mark_dirty(da7213->regmap);
> >> +       regulator_bulk_disable(DA7213_NUM_SUPPLIES, da7213->supplies);
> > 
> > There is no error checking.
> > We need to add error checking or return an error code.
> 
> This is just a move of da7213_runtime_suspend() to be able to use it in
> da7213_suspend() w/o forward declaration.

As this is just code move, I went ahead and applied the series.

Best regards,
								Pavel
-- 
In cooperation with DENX Software Engineering GmbH, HRB 165235 Munich,
Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[PATCH 5.10.y-cip 2/2] ASoC: renesas: rz-ssi: Use proper dma_buffer_pos after resume

[Claudiu]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

commit 22897e568646de5907d4981eae6cc895be2978d1 upstream.

When the driver supports DMA, it enqueues four DMA descriptors per
substream before the substream is started. New descriptors are enqueued in
the DMA completion callback, and each time a new descriptor is queued, the
dma_buffer_pos is incremented.

During suspend, the DMA transactions are terminated. There might be cases
where the four extra enqueued DMA descriptors are not completed and are
instead canceled on suspend. However, the cancel operation does not take
into account that the dma_buffer_pos was already incremented.

Previously, the suspend code reinitialized dma_buffer_pos to zero, but this
is not always correct.

To avoid losing any audio periods during suspend/resume and to prevent
clip sound, save the completed DMA buffer position in the DMA callback and
reinitialize dma_buffer_pos on resume.

Cc: stable@vger.kernel.org
Fixes: 1fc778f7c833a ("ASoC: renesas: rz-ssi: Add suspend to RAM support")
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Link: https://patch.msgid.link/20251029141134.2556926-3-claudiu.beznea.uj@bp.renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---
 sound/soc/sh/rz-ssi.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/sound/soc/sh/rz-ssi.c b/sound/soc/sh/rz-ssi.c
index c431e67903f7..f32ad81b08f8 100644
--- a/sound/soc/sh/rz-ssi.c
+++ b/sound/soc/sh/rz-ssi.c
@@ -86,6 +86,7 @@ struct rz_ssi_stream {
 	struct snd_pcm_substream *substream;
 	int fifo_sample_size;	/* sample capacity of SSI FIFO */
 	int dma_buffer_pos;	/* The address for the next DMA descriptor */
+	int completed_dma_buf_pos; /* The address of the last completed DMA descriptor. */
 	int period_counter;	/* for keeping track of periods transferred */
 	int sample_width;
 	int buffer_pos;		/* current frame position in the buffer */
@@ -230,6 +231,7 @@ static void rz_ssi_stream_init(struct rz_ssi_stream *strm,
 	rz_ssi_set_substream(strm, substream);
 	strm->sample_width = samples_to_bytes(runtime, 1);
 	strm->dma_buffer_pos = 0;
+	strm->completed_dma_buf_pos = 0;
 	strm->period_counter = 0;
 	strm->buffer_pos = 0;
 
@@ -454,6 +456,10 @@ static void rz_ssi_pointer_update(struct rz_ssi_stream *strm, int frames)
 		snd_pcm_period_elapsed(strm->substream);
 		strm->period_counter = current_period;
 	}
+
+	strm->completed_dma_buf_pos += runtime->period_size;
+	if (strm->completed_dma_buf_pos >= runtime->buffer_size)
+		strm->completed_dma_buf_pos = 0;
 }
 
 static int rz_ssi_pio_recv(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm)
@@ -793,10 +799,14 @@ static int rz_ssi_dma_request(struct rz_ssi_priv *ssi, struct device *dev)
 	return -ENODEV;
 }
 
-static int rz_ssi_trigger_resume(struct rz_ssi_priv *ssi)
+static int rz_ssi_trigger_resume(struct rz_ssi_priv *ssi, struct rz_ssi_stream *strm)
 {
+	struct snd_pcm_substream *substream = strm->substream;
+	struct snd_pcm_runtime *runtime = substream->runtime;
 	int ret;
 
+	strm->dma_buffer_pos = strm->completed_dma_buf_pos + runtime->period_size;
+
 	if (rz_ssi_is_stream_running(&ssi->playback) ||
 	    rz_ssi_is_stream_running(&ssi->capture))
 		return 0;
@@ -809,16 +819,6 @@ static int rz_ssi_trigger_resume(struct rz_ssi_priv *ssi)
 				ssi->hw_params_cache.channels);
 }
 
-static void rz_ssi_streams_suspend(struct rz_ssi_priv *ssi)
-{
-	if (rz_ssi_is_stream_running(&ssi->playback) ||
-	    rz_ssi_is_stream_running(&ssi->capture))
-		return;
-
-	ssi->playback.dma_buffer_pos = 0;
-	ssi->capture.dma_buffer_pos = 0;
-}
-
 static int rz_ssi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 			      struct snd_soc_dai *dai)
 {
@@ -828,7 +828,7 @@ static int rz_ssi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_RESUME:
-		ret = rz_ssi_trigger_resume(ssi);
+		ret = rz_ssi_trigger_resume(ssi, strm);
 		if (ret)
 			return ret;
 
@@ -867,7 +867,6 @@ static int rz_ssi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 		rz_ssi_stop(ssi, strm);
-		rz_ssi_streams_suspend(ssi);
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
-- 
2.43.0

Re: [PATCH 5.10.y-cip 2/2] ASoC: renesas: rz-ssi: Use proper dma_buffer_pos after resume

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

Hi!

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> commit 22897e568646de5907d4981eae6cc895be2978d1 upstream.
> 
> When the driver supports DMA, it enqueues four DMA descriptors per
> substream before the substream is started. New descriptors are enqueued in
> the DMA completion callback, and each time a new descriptor is queued, the
> dma_buffer_pos is incremented.
> 
> During suspend, the DMA transactions are terminated. There might be cases
> where the four extra enqueued DMA descriptors are not completed and are
> instead canceled on suspend. However, the cancel operation does not take
> into account that the dma_buffer_pos was already incremented.

Ok. So if you suspend the board in the middle of music playback, few
notes may be dropped or something? Don't do that, then. Suspending in
the middle of audio playback will sound horrible, anyway.

I guess this may result in some alert beep being lost (instead of
delayed), and I'm not against applying this, but...

Best regards,
								Pavel
-- 
In cooperation with DENX Software Engineering GmbH, HRB 165235 Munich,
Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH 5.10.y-cip 2/2] ASoC: renesas: rz-ssi: Use proper dma_buffer_pos after resume

[Claudiu Beznea]

Hi, Pavel,

On 11/24/25 12:21, Pavel Machek wrote:
> Hi!
> 
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> commit 22897e568646de5907d4981eae6cc895be2978d1 upstream.
>>
>> When the driver supports DMA, it enqueues four DMA descriptors per
>> substream before the substream is started. New descriptors are enqueued in
>> the DMA completion callback, and each time a new descriptor is queued, the
>> dma_buffer_pos is incremented.
>>
>> During suspend, the DMA transactions are terminated. There might be cases
>> where the four extra enqueued DMA descriptors are not completed and are
>> instead canceled on suspend. However, the cancel operation does not take
>> into account that the dma_buffer_pos was already incremented.
> 
> Ok. So if you suspend the board in the middle of music playback, few
> notes may be dropped or something?

Yes, few samples from the audio buffer will be dropped.

> Don't do that, then. Suspending in
> the middle of audio playback will sound horrible, anyway.

OK.

I think audio subsystem is trying its best to avoid glitches and other
unwanted sounds.

Thank you,
Claudiu

> 
> I guess this may result in some alert beep being lost (instead of
> delayed), and I'm not against applying this, but...
> 
> Best regards,
> 								Pavel

Re: [PATCH 5.10.y-cip 2/2] ASoC: renesas: rz-ssi: Use proper dma_buffer_pos after resume

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1527 bytes --]

On Mon 2025-11-24 15:23:41, Claudiu Beznea wrote:
> Hi, Pavel,
> 
> On 11/24/25 12:21, Pavel Machek wrote:
> > Hi!
> > 
> >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>
> >> commit 22897e568646de5907d4981eae6cc895be2978d1 upstream.
> >>
> >> When the driver supports DMA, it enqueues four DMA descriptors per
> >> substream before the substream is started. New descriptors are enqueued in
> >> the DMA completion callback, and each time a new descriptor is queued, the
> >> dma_buffer_pos is incremented.
> >>
> >> During suspend, the DMA transactions are terminated. There might be cases
> >> where the four extra enqueued DMA descriptors are not completed and are
> >> instead canceled on suspend. However, the cancel operation does not take
> >> into account that the dma_buffer_pos was already incremented.
> > 
> > Ok. So if you suspend the board in the middle of music playback, few
> > notes may be dropped or something?
> 
> Yes, few samples from the audio buffer will be dropped.
> 
> > Don't do that, then. Suspending in
> > the middle of audio playback will sound horrible, anyway.
> 
> OK.
> 
> I think audio subsystem is trying its best to avoid glitches and other
> unwanted sounds.

Yep, no problem, we can easily apply this. Its just... suspending
while audio is playing will not be ok either way.

Best regards,
								Pavel
-- 
In cooperation with DENX Software Engineering GmbH, HRB 165235 Munich,
Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]